From 62ba6196d40c12f56c34f38cecab061dfd4681cc Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 18 May 2025 13:52:12 -0700 Subject: [PATCH] sqlite: cleanup ERM support and export Session class Update sqlite Session to support Symbol.dispose and move the definition of the dispose methods to c++ to close the open TODO PR-URL: https://github.com/nodejs/node/pull/58378 Reviewed-By: Colin Ihrig Reviewed-By: Antoine du Hamel --- lib/sqlite.js | 15 +-------------- src/node_sqlite.cc | 20 ++++++++++++++++++++ src/node_sqlite.h | 2 ++ src/util.cc | 26 ++++++++++++++++++++++++++ src/util.h | 10 ++++++++++ test/parallel/test-sqlite-session.js | 15 +++++++++++++++ 6 files changed, 74 insertions(+), 14 deletions(-) diff --git a/lib/sqlite.js b/lib/sqlite.js index b011fd0921b..6d6ada72008 100644 --- a/lib/sqlite.js +++ b/lib/sqlite.js @@ -1,19 +1,6 @@ 'use strict'; -const { - SymbolDispose, -} = primordials; const { emitExperimentalWarning } = require('internal/util'); -const binding = internalBinding('sqlite'); emitExperimentalWarning('SQLite'); -// TODO(cjihrig): Move this to C++ once Symbol.dispose reaches Stage 4. -binding.DatabaseSync.prototype[SymbolDispose] = function() { - try { - this.close(); - } catch { - // Ignore errors. - } -}; - -module.exports = binding; +module.exports = internalBinding('sqlite'); diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 213b4cb155f..078c8d96ed2 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1005,6 +1005,14 @@ void DatabaseSync::Close(const FunctionCallbackInfo& args) { db->connection_ = nullptr; } +void DatabaseSync::Dispose(const v8::FunctionCallbackInfo& args) { + v8::TryCatch try_catch(args.GetIsolate()); + Close(args); + if (try_catch.HasCaught()) { + CHECK(try_catch.CanContinue()); + } +} + void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { DatabaseSync* db; ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); @@ -2577,6 +2585,7 @@ Local Session::GetConstructorTemplate(Environment* env) { SetProtoMethod( isolate, tmpl, "patchset", Session::Changeset); SetProtoMethod(isolate, tmpl, "close", Session::Close); + SetProtoDispose(isolate, tmpl, Session::Dispose); env->set_sqlite_session_constructor_template(tmpl); } return tmpl; @@ -2621,6 +2630,14 @@ void Session::Close(const FunctionCallbackInfo& args) { session->Delete(); } +void Session::Dispose(const v8::FunctionCallbackInfo& args) { + v8::TryCatch try_catch(args.GetIsolate()); + Close(args); + if (try_catch.HasCaught()) { + CHECK(try_catch.CanContinue()); + } +} + void Session::Delete() { if (!database_ || !database_->connection_ || session_ == nullptr) return; sqlite3session_delete(session_); @@ -2656,6 +2673,7 @@ static void Initialize(Local target, SetProtoMethod(isolate, db_tmpl, "open", DatabaseSync::Open); SetProtoMethod(isolate, db_tmpl, "close", DatabaseSync::Close); + SetProtoDispose(isolate, db_tmpl, DatabaseSync::Dispose); SetProtoMethod(isolate, db_tmpl, "prepare", DatabaseSync::Prepare); SetProtoMethod(isolate, db_tmpl, "exec", DatabaseSync::Exec); SetProtoMethod(isolate, db_tmpl, "function", DatabaseSync::CustomFunction); @@ -2686,6 +2704,8 @@ static void Initialize(Local target, target, "StatementSync", StatementSync::GetConstructorTemplate(env)); + SetConstructorFunction( + context, target, "Session", Session::GetConstructorTemplate(env)); target->Set(context, env->constants_string(), constants).Check(); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index eff62a97a8d..e02c1df9c2b 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -64,6 +64,7 @@ class DatabaseSync : public BaseObject { static void IsTransactionGetter( const v8::FunctionCallbackInfo& args); static void Close(const v8::FunctionCallbackInfo& args); + static void Dispose(const v8::FunctionCallbackInfo& args); static void Prepare(const v8::FunctionCallbackInfo& args); static void Exec(const v8::FunctionCallbackInfo& args); static void Location(const v8::FunctionCallbackInfo& args); @@ -194,6 +195,7 @@ class Session : public BaseObject { template static void Changeset(const v8::FunctionCallbackInfo& args); static void Close(const v8::FunctionCallbackInfo& args); + static void Dispose(const v8::FunctionCallbackInfo& args); static v8::Local GetConstructorTemplate( Environment* env); static BaseObjectPtr Create(Environment* env, diff --git a/src/util.cc b/src/util.cc index 78326b56eab..660cfff6b8a 100644 --- a/src/util.cc +++ b/src/util.cc @@ -598,6 +598,32 @@ void SetMethodNoSideEffect(Isolate* isolate, that->Set(name_string, t); } +void SetProtoDispose(v8::Isolate* isolate, + v8::Local that, + v8::FunctionCallback callback) { + Local signature = v8::Signature::New(isolate, that); + Local t = + NewFunctionTemplate(isolate, + callback, + signature, + v8::ConstructorBehavior::kThrow, + v8::SideEffectType::kHasSideEffect); + that->PrototypeTemplate()->Set(v8::Symbol::GetDispose(isolate), t); +} + +void SetProtoAsyncDispose(v8::Isolate* isolate, + v8::Local that, + v8::FunctionCallback callback) { + Local signature = v8::Signature::New(isolate, that); + Local t = + NewFunctionTemplate(isolate, + callback, + signature, + v8::ConstructorBehavior::kThrow, + v8::SideEffectType::kHasSideEffect); + that->PrototypeTemplate()->Set(v8::Symbol::GetAsyncDispose(isolate), t); +} + void SetProtoMethod(v8::Isolate* isolate, Local that, const std::string_view name, diff --git a/src/util.h b/src/util.h index 7527ec0909c..5d4be8c90ad 100644 --- a/src/util.h +++ b/src/util.h @@ -926,6 +926,16 @@ void SetMethodNoSideEffect(v8::Isolate* isolate, const std::string_view name, v8::FunctionCallback callback); +// Set the Symbol.dispose method on the prototype of the class. +void SetProtoDispose(v8::Isolate* isolate, + v8::Local that, + v8::FunctionCallback callback); + +// Set the Symbol.asyncDispose method on the prototype of the class. +void SetProtoAsyncDispose(v8::Isolate* isolate, + v8::Local that, + v8::FunctionCallback callback); + enum class SetConstructorFunctionFlag { NONE, SET_CLASS_NAME, diff --git a/test/parallel/test-sqlite-session.js b/test/parallel/test-sqlite-session.js index 121e27ec3e5..4ad2ebd5419 100644 --- a/test/parallel/test-sqlite-session.js +++ b/test/parallel/test-sqlite-session.js @@ -539,3 +539,18 @@ test('session.close() - closing twice', (t) => { message: 'session is not open' }); }); + +test('session supports ERM', (t) => { + const database = new DatabaseSync(':memory:'); + let afterDisposeSession; + { + using session = database.createSession(); + afterDisposeSession = session; + const changeset = session.changeset(); + t.assert.ok(changeset instanceof Uint8Array); + t.assert.strictEqual(changeset.length, 0); + } + t.assert.throws(() => afterDisposeSession.changeset(), { + message: /session is not open/, + }); +});