diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b4dfa3e..cbc9dcff 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/ext/sqlite3/database.c b/ext/sqlite3/database.c index 7d4072cc..734ca849 100644 --- a/ext/sqlite3/database.c +++ b/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); } diff --git a/test/test_resource_cleanup.rb b/test/test_resource_cleanup.rb index 51a4b2cf..5e75013e 100644 --- a/test/test_resource_cleanup.rb +++ b/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:')