diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 049fd786126fce..229443917fe6f0 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -349,6 +349,7 @@ class CustomAggregate { Global CustomAggregate::*mptr) { CustomAggregate* self = static_cast(sqlite3_user_data(ctx)); + CallbackDepthGuard guard(self->db_); Environment* env = self->env_; Isolate* isolate = env->isolate(); auto agg = self->GetAggregate(ctx); @@ -395,12 +396,18 @@ class CustomAggregate { return; } + if (!self->db_->IsOpen()) { + THROW_ERR_INVALID_STATE(env, "database is not open"); + return; + } + agg->value.Reset(isolate, ret); } static inline void xValueBase(sqlite3_context* ctx, bool is_final) { CustomAggregate* self = static_cast(sqlite3_user_data(ctx)); + CallbackDepthGuard guard(self->db_); Environment* env = self->env_; Isolate* isolate = env->isolate(); auto agg = self->GetAggregate(ctx); @@ -426,6 +433,9 @@ class CustomAggregate { .ToLocal(&result)) { self->db_->SetIgnoreNextSQLiteError(true); sqlite3_result_error(ctx, "", 0); + } else if (!self->db_->IsOpen()) { + THROW_ERR_INVALID_STATE(env, "database is not open"); + return; } } else { result = Local::New(isolate, agg->value); @@ -457,6 +467,10 @@ class CustomAggregate { auto fn = start_v.As(); MaybeLocal retval = fn->Call(env_->context(), Null(isolate), 0, nullptr); + if (!db_->IsOpen()) { + THROW_ERR_INVALID_STATE(env_, "database is not open"); + return nullptr; + } if (!retval.ToLocal(&start_v)) { db_->SetIgnoreNextSQLiteError(true); sqlite3_result_error(ctx, "", 0); @@ -669,6 +683,7 @@ void UserDefinedFunction::xFunc(sqlite3_context* ctx, sqlite3_value** argv) { UserDefinedFunction* self = static_cast(sqlite3_user_data(ctx)); + CallbackDepthGuard guard(self->db_); Environment* env = self->env_; Isolate* isolate = env->isolate(); auto recv = Undefined(isolate); @@ -700,6 +715,12 @@ void UserDefinedFunction::xFunc(sqlite3_context* ctx, MaybeLocal retval = fn->Call(env->context(), recv, argc, js_argv.data()); + + if (!self->db_->IsOpen()) { + THROW_ERR_INVALID_STATE(env, "database is not open"); + return; + } + Local result; if (!retval.ToLocal(&result)) { // Ignore the SQLite error because a JavaScript exception is pending. @@ -1434,6 +1455,8 @@ void DatabaseSync::Close(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_ON_BAD_STATE( + env, db->IsInCallback(), "database cannot be closed while in a callback"); db->FinalizeStatements(); db->DeleteSessions(); int r = sqlite3_close_v2(db->connection_); @@ -2359,13 +2382,17 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { } ArrayBufferViewContents buf(args[0]); - int r = sqlite3changeset_apply( - db->connection_, - buf.length(), - const_cast(static_cast(buf.data())), - context.filterCallback ? xFilter : nullptr, - xConflict, - static_cast(&context)); + int r; + { + CallbackDepthGuard guard(db); + r = sqlite3changeset_apply( + db->connection_, + buf.length(), + const_cast(static_cast(buf.data())), + context.filterCallback ? xFilter : nullptr, + xConflict, + static_cast(&context)); + } if (r == SQLITE_OK) { args.GetReturnValue().Set(true); return; @@ -2495,6 +2522,7 @@ int DatabaseSync::AuthorizerCallback(void* user_data, const char* param3, const char* param4) { DatabaseSync* db = static_cast(user_data); + CallbackDepthGuard guard(db); Environment* env = db->env(); Isolate* isolate = env->isolate(); HandleScope handle_scope(isolate); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index e7281ed266af5d..c586ed1968ddf9 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -228,6 +228,10 @@ class DatabaseSync : public BaseObject { void SetIgnoreNextSQLiteError(bool ignore); bool ShouldIgnoreSQLiteError(); + void IncrementCallbackDepth() { ++callback_depth_; } + void DecrementCallbackDepth() { --callback_depth_; } + bool IsInCallback() const { return callback_depth_ > 0; } + SET_MEMORY_INFO_NAME(DatabaseSync) SET_SELF_SIZE(DatabaseSync) @@ -241,6 +245,7 @@ class DatabaseSync : public BaseObject { bool enable_load_extension_; sqlite3* connection_; bool ignore_next_sqlite_error_; + int callback_depth_ = 0; std::set backups_; std::set sessions_; @@ -398,6 +403,19 @@ class SQLTagStore : public BaseObject { friend class StatementExecutionHelper; }; +class CallbackDepthGuard { + public: + explicit CallbackDepthGuard(DatabaseSync* db) : db_(db) { + db_->IncrementCallbackDepth(); + } + ~CallbackDepthGuard() { db_->DecrementCallbackDepth(); } + CallbackDepthGuard(const CallbackDepthGuard&) = delete; + CallbackDepthGuard& operator=(const CallbackDepthGuard&) = delete; + + private: + DatabaseSync* db_; +}; + class UserDefinedFunction { public: UserDefinedFunction(Environment* env, diff --git a/test/parallel/test-sqlite-udf-close.js b/test/parallel/test-sqlite-udf-close.js new file mode 100644 index 00000000000000..86794029b45713 --- /dev/null +++ b/test/parallel/test-sqlite-udf-close.js @@ -0,0 +1,39 @@ +'use strict'; + +const { skipIfSQLiteMissing } = require('../common'); +skipIfSQLiteMissing(); +const assert = require('node:assert'); +const { test } = require('node:test'); +const { DatabaseSync } = require('node:sqlite'); + +for (const method of ['all', 'get', 'run', 'iterate']) { + test(`database.close() from a UDF during statement.${method}()`, () => { + const db = new DatabaseSync(':memory:'); + db.exec(` + CREATE TABLE data (value INTEGER); + INSERT INTO data VALUES (1), (2), (3); + `); + + db.function('close_db', (value) => { + db.close(); + return value; + }); + + const statement = db.prepare('SELECT close_db(value) FROM data'); + assert.throws(() => { + if (method === 'iterate') { + for (const row of statement.iterate()) { + assert.ok(row); + } + } else { + statement[method](); + } + }, { + code: 'ERR_INVALID_STATE', + message: 'database cannot be closed while in a callback', + }); + + assert.strictEqual(db.isOpen, true); + db.close(); + }); +}