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

@@ -194,13 +194,21 @@
     if (castedThis->stmt == nullptr || castedThis->version_db == nullptr) [[unlikely]] {                           \
         throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Statement has finalized"_s)); \
         return {};                                                                                                 \
+    }                                                                                                              \
+    if (castedThis->version_db->finalizedStatementsOnClose) [[unlikely]] {                                         \
+        throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Database has closed"_s));     \
+        return {};                                                                                                 \
     }
 
 #define CHECK_PREPARED_JIT                                                                                         \
     if (castedThis->stmt == nullptr || castedThis->version_db == nullptr) [[unlikely]] {                           \
         throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Statement has finalized"_s)); \
         return {};                                                                                                 \
+    }                                                                                                              \
+    if (castedThis->version_db->finalizedStatementsOnClose) [[unlikely]] {                                         \
+        throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Database has closed"_s));     \
+        return {};                                                                                                 \
     }
 
 DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(VersionSqlite3);
 
@@ -217,6 +225,11 @@
     sqlite3* db;
     std::atomic<uint64_t> version;
     size_t reference_count;
+    // close() finalized every statement on this connection, so statement
+    // wrappers hold dangling sqlite3_stmt pointers they must never pass to
+    // sqlite again. Stays false on the sqlite3_close_v2() paths (GC release,
+    // termination), where outstanding statements remain valid.
+    bool finalizedStatementsOnClose = false;
 
     void release()
     {
@@ -937,14 +950,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() and free `stmt`, or close
+    // the database, which finalizes `stmt` via sqlite3_next_stmt().
+    // Re-validate before touching `stmt` again after any callback into JS.
     const auto& statementStillAlive = [&]() -> bool {
+        if (versionDB->finalizedStatementsOnClose) [[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 +1136,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 +1173,10 @@
         } else {
             value = array->getDirectIndex(lexicalGlobalObject, i);
             RETURN_IF_EXCEPTION(scope, {});
+            if (versionDB->finalizedStatementsOnClose) [[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,7 +1557,11 @@
                 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);
+                JSC::JSValue reb = rebindStatement(lexicalGlobalObject, bindingsAliveScope.value(), scope, db, versionDB, sql.stmt, bindings, safeIntegers, nullptr);
+                // A close() during binding finalized sql.stmt; keep the
+                // auto-destructor from finalizing it again.
+                if (versionDB->finalizedStatementsOnClose) [[unlikely]]
+                    sql.stmt = nullptr;
                 RETURN_IF_EXCEPTION(scope, {});
 
                 if (versionDB->db != db) [[unlikely]] {
@@ -1841,14 +1868,25 @@
         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 prepared statement so sqlite3_close() cannot
+    // fail with SQLITE_BUSY; wrappers see finalizedStatementsOnClose.
+    while (sqlite3_stmt* stmt = sqlite3_next_stmt(db, nullptr)) {
+        sqlite3_finalize(stmt);
+    }
+    versionDB->finalizedStatementsOnClose = true;
+
+    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());
 }
 
@@ -2254,6 +2292,10 @@
                         throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Statement has finalized"_s));
                         return {};
                     }
+                    if (castedThis->version_db->finalizedStatementsOnClose) [[unlikely]] {
+                        throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Database has closed"_s));
+                        return {};
+                    }
                     status = sqlite3_step(stmt);
                 } while (status == SQLITE_ROW);
             } else {
@@ -2266,6 +2308,10 @@
                         throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Statement has finalized"_s));
                         return {};
                     }
+                    if (castedThis->version_db->finalizedStatementsOnClose) [[unlikely]] {
+                        throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Database has closed"_s));
+                        return {};
+                    }
                     status = sqlite3_step(stmt);
                 } while (status == SQLITE_ROW);
             }
@@ -2412,6 +2458,10 @@
                         throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Statement has finalized"_s));
                         return {};
                     }
+                    if (castedThis->version_db->finalizedStatementsOnClose) [[unlikely]] {
+                        throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Database has closed"_s));
+                        return {};
+                    }
                     status = sqlite3_step(stmt);
                 } while (status == SQLITE_ROW);
             }
@@ -2494,16 +2544,18 @@
                         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));
                         return {};
                     }
+                    if (castedThis->version_db->finalizedStatementsOnClose) [[unlikely]] {
+                        throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Database has closed"_s));
+                        return {};
+                    }
 
                     status = sqlite3_step(stmt);
                 } while (status == SQLITE_ROW);
@@ -2607,6 +2659,12 @@
 
     CHECK_THIS
 
+    // After close() the stmt pointer is dangling; report the same empty
+    // string a finalized statement produces.
+    if (castedThis->version_db && castedThis->version_db->finalizedStatementsOnClose) [[unlikely]] {
+        RELEASE_AND_RETURN(scope, JSValue::encode(jsEmptyString(vm)));
+    }
+
     char* string = sqlite3_expanded_sql(castedThis->stmt);
     if (!string) {
         RELEASE_AND_RETURN(scope, JSValue::encode(jsEmptyString(vm)));
@@ -2823,7 +2881,9 @@
     CHECK_THIS
 
     if (castedThis->stmt) {
-        sqlite3_finalize(castedThis->stmt);
+        if (!(castedThis->version_db && castedThis->version_db->finalizedStatementsOnClose)) {
+            sqlite3_finalize(castedThis->stmt);
+        }
         castedThis->stmt = nullptr;
     }
 
@@ -2844,7 +2904,7 @@
 
 JSSQLStatement::~JSSQLStatement()
 {
-    if (this->stmt) {
+    if (this->stmt && !(this->version_db && this->version_db->finalizedStatementsOnClose)) {
         sqlite3_finalize(this->stmt);
     }
 
@@ -2873,7 +2933,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 +2943,13 @@
         return {};
     }
 
+    // A getter can also close the database, which finalizes `stmt` without
+    // nulling this wrapper's pointer.
+    if (this->version_db->finalizedStatementsOnClose) [[unlikely]] {
+        throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Database has closed"_s));
+        return {};
+    }
+
     if (val.isNumber()) {
         RELEASE_AND_RETURN(scope, val);
     } else {