Merge pull request #719 from katafrakt/close-after-failed-open

Close connection immediately after failed open

Author: flavorjones

CHANGELOG.md

@@ -1,5 +1,12 @@
 # sqlite3-ruby Changelog
 
+## next / unreleased
+
+### Improved
+
+- When `Database.new` fails to open the database file, the underlying sqlite3 connection handle is now closed immediately instead of waiting for the garbage collector to clean it up. #719 @katafrakt
+
+
 ## 2.9.5 / 2026-06-07
 
 ### Dependencies

ext/sqlite3/database.c

@@ -153,7 +153,13 @@ rb_sqlite3_open_v2(VALUE self, VALUE file, VALUE mode, VALUE zvfs)
                  NIL_P(zvfs) ? NULL : StringValuePtr(zvfs)
              );
 
-    CHECK(ctx->db, status);
+    if (status != SQLITE_OK) {
+        char *msg = sqlite3_mprintf("%s", sqlite3_errmsg(ctx->db));
+        sqlite3_close_v2(ctx->db);
+        ctx->db = NULL;
+        CHECK_MSG(ctx->db, status, msg);
+    }
+
     if (flags & SQLITE_OPEN_READONLY) {
         ctx->flags |= SQLITE3_RB_DATABASE_READONLY;
     }
@@ -941,7 +947,12 @@ rb_sqlite3_open16(VALUE self, VALUE file)
     // so we do not ever set SQLITE3_RB_DATABASE_READONLY in ctx->flags
     status = sqlite3_open16(utf16_string_value_ptr(file), &ctx->db);
 
-    CHECK(ctx->db, status)
+    if (status != SQLITE_OK) {
+        char *msg = sqlite3_mprintf("%s", sqlite3_errmsg(ctx->db));
+        sqlite3_close_v2(ctx->db);
+        ctx->db = NULL;
+        CHECK_MSG(ctx->db, status, msg);
+    }
 
     return INT2NUM(status);
 }

test/test_resource_cleanup.rb

@@ -17,6 +17,28 @@ def test_cleanup_unclosed_statement_object
       end
     end
 
+    # These tests exercise the failure branches of open_v2 and open16, which immediately close the
+    # sqlite3 handle and reclaim resources before raising.
+    #
+    # Under valgrind these would catch any memory problems in the close-on-failure code, but they do
+    # NOT directly test that resources are immediately recovered. If we didn't close immediately,
+    # the GC finalizer would still eventually close any leftover handles if and when the database
+    # object is GCed. Beacuse we run ruby with free-at-exit with valgrind, directly testing the
+    # immediate close is hard. But I'm fine with that tradeoff.
+    def test_cleanup_failed_open_v2
+      exception = assert_raise(SQLite3::CantOpenException) do
+        SQLite3::Database.new("/nonexistent/foo.db")
+      end
+      assert_equal("unable to open database file", exception.message)
+    end
+
+    def test_cleanup_failed_open16
+      exception = assert_raise(SQLite3::CantOpenException) do
+        SQLite3::Database.new("/nonexistent/foo.db".encode(Encoding::UTF_16LE))
+      end
+      assert_equal("unable to open database file", exception.message)
+    end
+
     # # this leaks the result set
     # def test_cleanup_unclosed_resultset_object
     #   db = SQLite3::Database.new(':memory:')