commit d8ce941f98ccb279a7706fddd3a634e2db2e3aa3
parent 7b4ee7114e123a0fedb34d5086d533dec109c896
Author: Florian Dold <dold@taler.net>
Date: Sat, 29 Aug 2026 12:28:40 +0200
reject closing SQLite connections with live statements
Diffstat:
4 files changed, 223 insertions(+), 18 deletions(-)
diff --git a/meson.build b/meson.build
@@ -304,6 +304,8 @@ if not meson.is_cross_build()
args : [files('tests/test_prelude.js')])
test('sqlite3', qtart_exe,
args : [files('tests/test_sqlite3_error.js')])
+ test('wallet-sqlite-lifecycle', qtart_exe,
+ args : [files('tests/test_wallet_sqlite_lifecycle.js')])
test('tart-inputs', qtart_exe,
args : [files('tests/test_tart_inputs.js')])
test('wallet-lifecycle', wallet_lifecycle_test,
diff --git a/tart_module.c b/tart_module.c
@@ -1581,7 +1581,9 @@ static void js_sqlite3_database_finalizer(JSRuntime *rt, JSValue val)
{
sqlite3 *sqlite3_db;
sqlite3_db = JS_GetOpaque(val, js_sqlite3_database_class_id);
- (void)sqlite3_close_v2(sqlite3_db);
+ if (sqlite3_db) {
+ (void)sqlite3_close_v2(sqlite3_db);
+ }
JS_SetOpaque(val, NULL);
}
@@ -1590,8 +1592,11 @@ static void js_sqlite3_statement_finalizer(JSRuntime *rt, JSValue val)
sqlite3_stmt *stmt;
stmt = JS_GetOpaque(val, js_sqlite3_statement_class_id);
- // FIXME: Check error code and warn?
- sqlite3_finalize(stmt);
+ if (stmt) {
+ // The return code reports the last evaluation error, not whether the
+ // statement resource was released, so there is nothing to retry here.
+ (void)sqlite3_finalize(stmt);
+ }
JS_SetOpaque(val, NULL);
}
@@ -1835,7 +1840,6 @@ static JSValue throw_sqlite3_error(JSContext *ctx, sqlite3 *db)
return JS_Throw(ctx, obj);
}
-
#define MAX_SAFE_INTEGER (((int64_t)1 << 53) - 1)
#define MIN_SAFE_INTEGER (-(((int64_t)1 << 53) - 1))
@@ -1895,8 +1899,15 @@ done:
static JSValue js_sqlite3_close(JSContext *ctx, JSValue this_val,
int argc, JSValueConst *argv)
{
- JSValue db_handle = argv[0];
+ JSValue db_handle;
sqlite3 *sqlite3_db;
+ int ret;
+
+ if (argc < 1 ||
+ JS_GetClassID(argv[0]) != js_sqlite3_database_class_id) {
+ return JS_ThrowTypeError(ctx, "expected sqlite3 database handle");
+ }
+ db_handle = argv[0];
sqlite3_db = JS_GetOpaque(db_handle, js_sqlite3_database_class_id);
@@ -1904,7 +1915,15 @@ static JSValue js_sqlite3_close(JSContext *ctx, JSValue this_val,
return JS_ThrowTypeError(ctx, "invalid sqlite3 database handle");
}
- (void) sqlite3_close_v2(sqlite3_db);
+ /*
+ * Unlike close_v2(), sqlite3_close() refuses to turn a connection with
+ * live statements into a zombie. Keep the JS handle valid on SQLITE_BUSY
+ * so the caller can finalize the statements and retry safely.
+ */
+ ret = sqlite3_close(sqlite3_db);
+ if (SQLITE_OK != ret) {
+ return throw_sqlite3_error(ctx, sqlite3_db);
+ }
JS_SetOpaque(db_handle, NULL);
return JS_UNDEFINED;
}
@@ -1913,8 +1932,8 @@ static JSValue js_sqlite3_close(JSContext *ctx, JSValue this_val,
static JSValue js_sqlite3_prepare(JSContext *ctx, JSValue this_val,
int argc, JSValueConst *argv)
{
- JSValue db_handle = argv[0];
- JSValue stmt_str = argv[1];
+ JSValue db_handle;
+ JSValue stmt_str;
JSValue ret_val = JS_UNDEFINED;
JSValue stmt_obj = JS_UNDEFINED;
int ret;
@@ -1923,6 +1942,13 @@ static JSValue js_sqlite3_prepare(JSContext *ctx, JSValue this_val,
const char *stmt_cstr;
const char *tail;
+ if (argc < 2 ||
+ JS_GetClassID(argv[0]) != js_sqlite3_database_class_id) {
+ return JS_ThrowTypeError(ctx, "expected database handle and SQL string");
+ }
+ db_handle = argv[0];
+ stmt_str = argv[1];
+
sqlite3_db = JS_GetOpaque(db_handle, js_sqlite3_database_class_id);
if (!sqlite3_db) {
@@ -1949,6 +1975,21 @@ static JSValue js_sqlite3_prepare(JSContext *ctx, JSValue this_val,
goto done;
}
JS_SetOpaque(stmt_obj, stmt);
+ /*
+ * A statement must keep its database JS object alive. Otherwise QuickJS
+ * could run the database finalizer while the sqlite3_stmt still refers to
+ * that connection.
+ */
+ if (JS_DefinePropertyValueStr(
+ ctx,
+ stmt_obj,
+ "__sqlite3_database_owner",
+ JS_DupValue(ctx, db_handle),
+ 0) < 0) {
+ JS_FreeValue(ctx, stmt_obj);
+ ret_val = JS_EXCEPTION;
+ goto done;
+ }
ret_val = stmt_obj;
done:
JS_FreeCString(ctx, stmt_cstr);
@@ -1961,12 +2002,15 @@ static JSValue js_sqlite3_finalize(JSContext *ctx, JSValue this_val,
{
sqlite3_stmt *stmt;
+ if (argc < 1 ||
+ JS_GetClassID(argv[0]) != js_sqlite3_statement_class_id) {
+ return JS_ThrowTypeError(ctx, "expected sqlite3 statement handle");
+ }
stmt = JS_GetOpaque(argv[0], js_sqlite3_statement_class_id);
if (!stmt) {
- return JS_ThrowTypeError(ctx, "unable to finalize (not a statement)");
+ return JS_UNDEFINED;
}
- // FIXME: Check error code and warn?
- sqlite3_finalize(stmt);
+ (void)sqlite3_finalize(stmt);
JS_SetOpaque(argv[0], NULL);
return JS_UNDEFINED;
}
@@ -2151,14 +2195,20 @@ static JSValue js_sqlite3_stmt_run(JSContext *ctx, JSValue this_val,
int argc, JSValueConst *argv)
{
JSValue ret_val = JS_UNDEFINED;
- JSValue stmt_handle = argv[0];
+ JSValue stmt_handle;
sqlite3_stmt *stmt;
sqlite3 *db;
int sqlret;
+ if (argc < 1 ||
+ JS_GetClassID(argv[0]) != js_sqlite3_statement_class_id) {
+ return JS_ThrowTypeError(ctx, "expected sqlite3 statement handle");
+ }
+ stmt_handle = argv[0];
+
stmt = JS_GetOpaque(stmt_handle, js_sqlite3_statement_class_id);
if (!stmt) {
- ret_val = JS_ThrowTypeError(ctx, "invalid sqlite3 database handle");
+ ret_val = JS_ThrowTypeError(ctx, "invalid sqlite3 statement handle");
goto done;
}
db = sqlite3_db_handle(stmt);
@@ -2271,15 +2321,21 @@ static JSValue js_sqlite3_stmt_get_all(JSContext *ctx, JSValue this_val,
int argc, JSValueConst *argv)
{
JSValue ret_val = JS_UNDEFINED;
- JSValue stmt_handle = argv[0];
+ JSValue stmt_handle;
sqlite3_stmt *stmt;
sqlite3 *db;
int sqlret;
JSValue rows_array = JS_UNDEFINED;
+ if (argc < 1 ||
+ JS_GetClassID(argv[0]) != js_sqlite3_statement_class_id) {
+ return JS_ThrowTypeError(ctx, "expected sqlite3 statement handle");
+ }
+ stmt_handle = argv[0];
+
stmt = JS_GetOpaque(stmt_handle, js_sqlite3_statement_class_id);
if (!stmt) {
- ret_val = JS_ThrowTypeError(ctx, "invalid sqlite3 database handle");
+ ret_val = JS_ThrowTypeError(ctx, "invalid sqlite3 statement handle");
goto done;
}
db = sqlite3_db_handle(stmt);
@@ -2339,15 +2395,21 @@ fail:
static JSValue js_sqlite3_stmt_get_first(JSContext *ctx, JSValue this_val,
int argc, JSValueConst *argv)
{
- JSValue ret_val = JS_UNDEFINED;
- JSValue stmt_handle = argv[0];
+ JSValue ret_val = JS_UNDEFINED;
+ JSValue stmt_handle;
sqlite3_stmt *stmt;
sqlite3 *db;
int sqlret;
+ if (argc < 1 ||
+ JS_GetClassID(argv[0]) != js_sqlite3_statement_class_id) {
+ return JS_ThrowTypeError(ctx, "expected sqlite3 statement handle");
+ }
+ stmt_handle = argv[0];
+
stmt = JS_GetOpaque(stmt_handle, js_sqlite3_statement_class_id);
if (!stmt) {
- ret_val = JS_ThrowTypeError(ctx, "invalid sqlite3 database handle");
+ ret_val = JS_ThrowTypeError(ctx, "invalid sqlite3 statement handle");
goto done;
}
db = sqlite3_db_handle(stmt);
diff --git a/tests/test_sqlite3_error.js b/tests/test_sqlite3_error.js
@@ -31,6 +31,19 @@ function expectSqlite3Error(fn, code, messagePart) {
throw new Error(`expected ${code} Sqlite3Error`);
}
+function expectTypeError(fn, messagePart) {
+ try {
+ fn();
+ } catch (error) {
+ assert(error instanceof TypeError,
+ `expected TypeError, got ${error?.constructor?.name}`);
+ assert(error.message.includes(messagePart),
+ `unexpected TypeError message: ${error.message}`);
+ return;
+ }
+ throw new Error("expected TypeError");
+}
+
try {
db = tart.sqlite3Open(dbPath);
tart.sqlite3Exec(db, "CREATE TABLE entries (value TEXT UNIQUE)");
@@ -56,6 +69,26 @@ try {
// Changing into WAL fails while another statement on the connection is
// still active. sqlite3StmtGetFirst must reset its SELECT before returning.
tart.sqlite3Exec(db, "PRAGMA journal_mode = WAL");
+
+ // Explicit close must never create a zombie connection. A direct caller
+ // that leaves a statement live gets SQLITE_BUSY and can still use and
+ // finalize the statement before retrying the close.
+ expectSqlite3Error(
+ () => tart.sqlite3Close(db),
+ "SQLITE_BUSY",
+ "unable to close",
+ );
+ assert(tart.sqlite3StmtGetFirst(stmt)?.value === "one",
+ "statement must remain usable after SQLITE_BUSY from close");
+ tart.sqlite3Finalize(stmt);
+ tart.sqlite3Finalize(stmt);
+ expectTypeError(
+ () => tart.sqlite3StmtGetFirst(stmt),
+ "invalid sqlite3 statement handle",
+ );
+ stmt = undefined;
+ tart.sqlite3Close(db);
+ db = undefined;
} finally {
if (stmt !== undefined) {
tart.sqlite3Finalize(stmt);
diff --git a/tests/test_wallet_sqlite_lifecycle.js b/tests/test_wallet_sqlite_lifecycle.js
@@ -0,0 +1,108 @@
+import * as os from "os";
+
+function assert(condition, message) {
+ if (!condition) throw new Error(message);
+}
+
+const dbPath = `/tmp/qtart-wallet-lifecycle-${Date.now()}.sqlite3`;
+const idbDbPath = `/tmp/qtart-wallet-idb-lifecycle-${Date.now()}.sqlite3`;
+const { createNativeWalletHost2 } = globalThis.talerModules.talerWalletCore;
+
+try {
+ const { wallet } = await createNativeWalletHost2({
+ persistentStoragePath: dbPath,
+ });
+ const init = await wallet.handleCoreApiRequest("initWallet", "init", {
+ config: {
+ lazyTaskLoop: true,
+ testing: { skipDefaults: true },
+ features: { migrateNativeDb: false, useNativeDb: true },
+ },
+ });
+ assert(init.type === "response", `init failed: ${JSON.stringify(init)}`);
+ assert(
+ init.result.databaseBackend === "sqlite",
+ `unexpected backend: ${JSON.stringify(init.result)}`,
+ );
+
+ const balances = await wallet.handleCoreApiRequest(
+ "getBalances",
+ "balances",
+ {},
+ );
+ assert(
+ balances.type === "response",
+ `balance query failed: ${JSON.stringify(balances)}`,
+ );
+
+ const shutdown = await wallet.handleCoreApiRequest(
+ "shutdown",
+ "shutdown",
+ {},
+ );
+ assert(
+ shutdown.type === "response",
+ `shutdown failed: ${JSON.stringify(shutdown)}`,
+ );
+ const repeated = await wallet.handleCoreApiRequest(
+ "shutdown",
+ "shutdown-again",
+ {},
+ );
+ assert(
+ repeated.type === "response",
+ `repeated shutdown failed: ${JSON.stringify(repeated)}`,
+ );
+
+ try {
+ await wallet.handleCoreApiRequest("getBalances", "after-shutdown", {});
+ throw new Error("request after shutdown unexpectedly succeeded");
+ } catch (error) {
+ assert(
+ error?.errorDetail?.code === 7011,
+ `unexpected post-shutdown error: ${error?.stack ?? error}`,
+ );
+ }
+
+ // The legacy IndexedDB emulation owns the same kind of raw SQLite
+ // connection. Its terminal close must dispose the backend statement cache
+ // before releasing that connection too.
+ const { wallet: idbWallet } = await createNativeWalletHost2({
+ persistentStoragePath: idbDbPath,
+ });
+ const idbInit = await idbWallet.handleCoreApiRequest(
+ "initWallet",
+ "idb-init",
+ {
+ config: {
+ lazyTaskLoop: true,
+ testing: { skipDefaults: true },
+ features: { migrateNativeDb: false, useNativeDb: false },
+ },
+ },
+ );
+ assert(
+ idbInit.type === "response",
+ `IDB init failed: ${JSON.stringify(idbInit)}`,
+ );
+ assert(
+ idbInit.result.databaseBackend === "indexeddb",
+ `unexpected IDB backend: ${JSON.stringify(idbInit.result)}`,
+ );
+ const idbShutdown = await idbWallet.handleCoreApiRequest(
+ "shutdown",
+ "idb-shutdown",
+ {},
+ );
+ assert(
+ idbShutdown.type === "response",
+ `IDB shutdown failed: ${JSON.stringify(idbShutdown)}`,
+ );
+} finally {
+ os.remove(dbPath);
+ os.remove(`${dbPath}-wal`);
+ os.remove(`${dbPath}-shm`);
+ os.remove(idbDbPath);
+ os.remove(`${idbDbPath}-wal`);
+ os.remove(`${idbDbPath}-shm`);
+}
