bun:sqlite: finalize outstanding prepared statements on close via sqlite3_next_stmt

docs/runtime/sqlite.mdx

@@ -112,15 +112,15 @@ const db = new Database("./mydb.sqlite");
 
 ### `.close(throwOnError: boolean = false)`
 
-To close a database connection but allow existing queries to finish, call `.close(false)`:
+To close a database connection, call `.close()`:
 
 ```ts db.ts icon="/icons/typescript.svg" highlight={3}
 const db = new Database();
 // ... do stuff
-db.close(false);
+db.close();
 ```
 
-To close the database and throw an error if there are any pending queries, call `.close(true)`:
+Any prepared statements that were not finalized are finalized as part of closing, so the connection (and the database file) is released immediately. Using a statement after its database was closed throws an error, except `toString()`, which returns an empty string, and `finalize()`, which stays safe to call. Pass `true` to throw if the connection fails to close:
 
 ```ts db.ts icon="/icons/typescript.svg" highlight={3}
 const db = new Database();
@@ -129,8 +129,8 @@ db.close(true);
 ```
 
 <Note>
-  `close(false)` is called automatically when the database is garbage collected. It is safe to call multiple times but
-  has no effect after the first.
+  `close()` is called automatically when the database is garbage collected. It is safe to call multiple times but has no
+  effect after the first.
 </Note>
 
 ### `using` statement

packages/bun-types/sqlite.d.ts

@@ -264,25 +264,26 @@ declare module "bun:sqlite" {
     /**
      * Close the database connection.
      *
-     * It is safe to call this method multiple times. If the database is already
-     * closed, this is a no-op. Running queries after the database has been
-     * closed throws an error.
+     * Prepared statements that were not finalized are finalized as part of
+     * closing, so the connection is released immediately. It is safe to call
+     * this method multiple times. If the database is already closed, this is
+     * a no-op. Running queries or using statements after the database has
+     * been closed throws an error, except statement `toString()`, which
+     * returns an empty string, and `finalize()`, which stays safe to call.
      *
      * @example
      * ```ts
      * db.close();
      * ```
      * This is called automatically when the database instance is garbage collected.
      *
-     * Internally, this calls `sqlite3_close_v2`.
+     * Internally, this calls `sqlite3_close`.
      */
     close(
       /**
-       * If `true`, throw an error if the database is in use
+       * If `true`, throw an error if the connection fails to close
        * @default false
        *
-       * When `true`, this calls `sqlite3_close` instead of `sqlite3_close_v2`.
-       *
        * Learn more in the [sqlite3 documentation](https://www.sqlite.org/c3ref/close.html).
        *
        * In the future, Bun may default `throwOnError` to `true`, but for backwards compatibility it is `false` by default.

src/jsc/bindings/sqlite/JSSQLStatement.cpp

@@ -196,11 +196,9 @@
         return {};                                                                                                 \
     }
 
-#define CHECK_PREPARED_JIT                                                                                         \
-    if (castedThis->stmt == nullptr || castedThis->version_db == nullptr) [[unlikely]] {                           \
-        throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Statement has finalized"_s)); \
-        return {};                                                                                                 \
-    }
+namespace WebCore {
+class JSSQLStatement;
+}
 
 DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(VersionSqlite3);
 
@@ -217,6 +215,10 @@
     sqlite3* db;
     std::atomic<uint64_t> version;
     size_t reference_count;
+    // Live statement wrappers on this connection. close() finalizes each one
+    // and nulls its stmt pointer, like better-sqlite3's CloseHandles() and
+    // node:sqlite's FinalizeStatements().
+    WTF::Vector<WebCore::JSSQLStatement*> statements;
 
     void release()
     {
@@ -475,6 +477,7 @@
         JSSQLStatement* ptr = new (NotNull, JSC::allocateCell<JSSQLStatement>(globalObject->vm())) JSSQLStatement(structure, *globalObject, stmt, version_db, memorySizeChange);
         if (version_db) {
             ++version_db->reference_count;
+            version_db->statements.append(ptr);
         }
         ptr->finishCreation(globalObject->vm());
         return ptr;
@@ -937,14 +940,20 @@
 #undef CHECK_BIND
 }
 
-static JSC::JSValue rebindObject(JSC::JSGlobalObject* globalObject, SQLiteBindingsMap& bindings, JSC::JSObject* target, JSC::ThrowScope& scope, sqlite3* db, sqlite3_stmt* stmt, bool safeIntegers, JSSQLStatement* statement)
+static JSC::JSValue rebindObject(JSC::JSGlobalObject* globalObject, SQLiteBindingsMap& bindings, JSC::JSObject* target, JSC::ThrowScope& scope, sqlite3* db, VersionSqlite3* versionDB, sqlite3_stmt* stmt, bool safeIntegers, JSSQLStatement* statement)
 {
     int count = 0;
 
     // Reading a property off `target` can run arbitrary JS (getters, Proxy
-    // traps), which can call statement.finalize() and free `stmt`. Re-validate
-    // before touching `stmt` again after any callback into JS.
+    // traps), which can call statement.finalize() or db.close() and free
+    // `stmt`. Re-validate before touching `stmt` again after any callback
+    // into JS.
     const auto& statementStillAlive = [&]() -> bool {
+        if (versionDB->db != db) [[unlikely]] {
+            if (!scope.exception())
+                throwException(globalObject, scope, createError(globalObject, "Database has closed"_s));
+            return false;
+        }
         if (statement && statement->stmt != stmt) [[unlikely]] {
             if (!scope.exception())
                 throwException(globalObject, scope, createError(globalObject, "Statement has finalized"_s));
@@ -1117,15 +1126,15 @@
     return jsNumber(count);
 }
 
-static JSC::JSValue rebindStatement(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue values, JSC::ThrowScope& scope, sqlite3* db, sqlite3_stmt* stmt, SQLiteBindingsMap& bindings, bool safeIntegers, JSSQLStatement* statement)
+static JSC::JSValue rebindStatement(JSC::JSGlobalObject* lexicalGlobalObject, JSC::JSValue values, JSC::ThrowScope& scope, sqlite3* db, VersionSqlite3* versionDB, sqlite3_stmt* stmt, SQLiteBindingsMap& bindings, bool safeIntegers, JSSQLStatement* statement)
 {
     sqlite3_clear_bindings(stmt);
     JSC::JSArray* array = dynamicDowncast<JSC::JSArray>(values);
     bindings.reset(sqlite3_bind_parameter_count(stmt));
 
     if (!array) {
         if (JSC::JSObject* object = values.getObject()) {
-            auto res = rebindObject(lexicalGlobalObject, bindings, object, scope, db, stmt, safeIntegers, statement);
+            auto res = rebindObject(lexicalGlobalObject, bindings, object, scope, db, versionDB, stmt, safeIntegers, statement);
             RETURN_IF_EXCEPTION(scope, {});
             return res;
         }
@@ -1154,6 +1163,10 @@
         } else {
             value = array->getDirectIndex(lexicalGlobalObject, i);
             RETURN_IF_EXCEPTION(scope, {});
+            if (versionDB->db != db) [[unlikely]] {
+                throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Database has closed"_s));
+                return {};
+            }
             if (statement && statement->stmt != stmt) [[unlikely]] {
                 throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Statement has finalized"_s));
                 return {};
@@ -1534,13 +1547,17 @@
                 int count = sqlite3_bind_parameter_count(sql.stmt);
 
                 SQLiteBindingsMap bindings { static_cast<uint16_t>(count > -1 ? count : 0), strict };
-                JSC::JSValue reb = rebindStatement(lexicalGlobalObject, bindingsAliveScope.value(), scope, db, sql.stmt, bindings, safeIntegers, nullptr);
-                RETURN_IF_EXCEPTION(scope, {});
-
+                JSC::JSValue reb = rebindStatement(lexicalGlobalObject, bindingsAliveScope.value(), scope, db, versionDB, sql.stmt, bindings, safeIntegers, nullptr);
                 if (versionDB->db != db) [[unlikely]] {
-                    throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Database has closed"_s));
+                    // A close() during binding finalized sql.stmt via its
+                    // sqlite3_next_stmt() sweep; keep the auto-destructor
+                    // from finalizing it again.
+                    sql.stmt = nullptr;
+                    if (!scope.exception())
+                        throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Database has closed"_s));
                     return {};
                 }
+                RETURN_IF_EXCEPTION(scope, {});
 
                 if (!reb.isNumber()) [[unlikely]] {
                     return JSValue::encode(reb); /* this means an error */
@@ -1841,14 +1858,35 @@
         return JSValue::encode(jsUndefined());
     }
 
-    // sqlite3_close_v2 is used for automatic GC cleanup
-    int statusCode = shouldThrowOnError ? sqlite3_close(db) : sqlite3_close_v2(db);
+    // Finalize every outstanding statement so sqlite3_close() cannot fail
+    // with SQLITE_BUSY, nulling each wrapper's handle so it never touches
+    // freed memory.
+    for (auto* statement : versionDB->statements) {
+        if (statement->stmt) {
+            sqlite3_finalize(statement->stmt);
+            statement->stmt = nullptr;
+        }
+    }
+    versionDB->statements.clear();
+
+    // Backstop for statements not owned by a wrapper, e.g. the transient one
+    // in jsSQLStatementExecuteFunction when a bound getter calls close().
+    while (sqlite3_stmt* stmt = sqlite3_next_stmt(db, nullptr)) {
+        sqlite3_finalize(stmt);
+    }
+
+    int statusCode = sqlite3_close(db);
     if (statusCode != SQLITE_OK) {
+        // The statements are already gone, so the connection is unusable
+        // either way: defer the close and retire the handle.
+        sqlite3_close_v2(db);
+    }
+    versionDB->db = nullptr;
+
+    if (statusCode != SQLITE_OK && shouldThrowOnError) {
         throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, WTF::String::fromUTF8(sqlite3_errstr(statusCode))));
         return {};
     }
-
-    versionDB->db = nullptr;
     return JSValue::encode(jsUndefined());
 }
 
@@ -2494,11 +2532,9 @@
                         RELEASE_AND_RETURN(scope, {});
                     }
                     resultArray->push(lexicalGlobalObject, row);
-
-                    if (scope.exception()) [[unlikely]] {
-                        sqlite3_reset(stmt);
-                        RELEASE_AND_RETURN(scope, {});
-                    }
+                    // No sqlite3_reset here: push() can run user code that
+                    // finalizes `stmt` (statement.finalize() or db.close()).
+                    RETURN_IF_EXCEPTION(scope, {});
 
                     if (castedThis->stmt != stmt) [[unlikely]] {
                         throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Statement has finalized"_s));
@@ -2844,6 +2880,9 @@
 
 JSSQLStatement::~JSSQLStatement()
 {
+    if (this->version_db) {
+        this->version_db->statements.removeFirst(this);
+    }
     if (this->stmt) {
         sqlite3_finalize(this->stmt);
     }
@@ -2873,7 +2912,7 @@
     auto scope = DECLARE_THROW_SCOPE(vm);
     auto* stmt = this->stmt;
 
-    auto val = rebindStatement(lexicalGlobalObject, values, scope, this->version_db->db, stmt, this->m_bindingNames, this->useBigInt64, this);
+    auto val = rebindStatement(lexicalGlobalObject, values, scope, this->version_db->db, this->version_db, stmt, this->m_bindingNames, this->useBigInt64, this);
     RETURN_IF_EXCEPTION(scope, {});
 
     // A getter invoked while binding can finalize this statement; the callers
@@ -2883,6 +2922,8 @@
         return {};
     }
 
+
+
     if (val.isNumber()) {
         RELEASE_AND_RETURN(scope, val);
     } else {