Skip to content

Commit a6456ab

Browse files
jasnelltargos
authored andcommitted
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: #58378 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent fa0188a commit a6456ab

File tree

6 files changed

+74
-14
lines changed

6 files changed

+74
-14
lines changed

lib/sqlite.js

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,6 @@
11
'use strict';
2-
const {
3-
SymbolDispose,
4-
} = primordials;
52
const { emitExperimentalWarning } = require('internal/util');
6-
const binding = internalBinding('sqlite');
73

84
emitExperimentalWarning('SQLite');
95

10-
// TODO(cjihrig): Move this to C++ once Symbol.dispose reaches Stage 4.
11-
binding.DatabaseSync.prototype[SymbolDispose] = function() {
12-
try {
13-
this.close();
14-
} catch {
15-
// Ignore errors.
16-
}
17-
};
18-
19-
module.exports = binding;
6+
module.exports = internalBinding('sqlite');

src/node_sqlite.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,14 @@ void DatabaseSync::Close(const FunctionCallbackInfo<Value>& args) {
11031103
db->connection_ = nullptr;
11041104
}
11051105

1106+
void DatabaseSync::Dispose(const v8::FunctionCallbackInfo<v8::Value>& args) {
1107+
v8::TryCatch try_catch(args.GetIsolate());
1108+
Close(args);
1109+
if (try_catch.HasCaught()) {
1110+
CHECK(try_catch.CanContinue());
1111+
}
1112+
}
1113+
11061114
void DatabaseSync::Prepare(const FunctionCallbackInfo<Value>& args) {
11071115
DatabaseSync* db;
11081116
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
@@ -3015,6 +3023,7 @@ Local<FunctionTemplate> Session::GetConstructorTemplate(Environment* env) {
30153023
SetProtoMethod(
30163024
isolate, tmpl, "patchset", Session::Changeset<sqlite3session_patchset>);
30173025
SetProtoMethod(isolate, tmpl, "close", Session::Close);
3026+
SetProtoDispose(isolate, tmpl, Session::Dispose);
30183027
env->set_sqlite_session_constructor_template(tmpl);
30193028
}
30203029
return tmpl;
@@ -3059,6 +3068,14 @@ void Session::Close(const FunctionCallbackInfo<Value>& args) {
30593068
session->Delete();
30603069
}
30613070

3071+
void Session::Dispose(const v8::FunctionCallbackInfo<v8::Value>& args) {
3072+
v8::TryCatch try_catch(args.GetIsolate());
3073+
Close(args);
3074+
if (try_catch.HasCaught()) {
3075+
CHECK(try_catch.CanContinue());
3076+
}
3077+
}
3078+
30623079
void Session::Delete() {
30633080
if (!database_ || !database_->connection_ || session_ == nullptr) return;
30643081
sqlite3session_delete(session_);
@@ -3094,6 +3111,7 @@ static void Initialize(Local<Object> target,
30943111

30953112
SetProtoMethod(isolate, db_tmpl, "open", DatabaseSync::Open);
30963113
SetProtoMethod(isolate, db_tmpl, "close", DatabaseSync::Close);
3114+
SetProtoDispose(isolate, db_tmpl, DatabaseSync::Dispose);
30973115
SetProtoMethod(isolate, db_tmpl, "prepare", DatabaseSync::Prepare);
30983116
SetProtoMethod(isolate, db_tmpl, "exec", DatabaseSync::Exec);
30993117
SetProtoMethod(isolate, db_tmpl, "function", DatabaseSync::CustomFunction);
@@ -3133,6 +3151,8 @@ static void Initialize(Local<Object> target,
31333151
target,
31343152
"StatementSync",
31353153
StatementSync::GetConstructorTemplate(env));
3154+
SetConstructorFunction(
3155+
context, target, "Session", Session::GetConstructorTemplate(env));
31363156

31373157
target->Set(context, env->constants_string(), constants).Check();
31383158

src/node_sqlite.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ class DatabaseSync : public BaseObject {
123123
static void IsTransactionGetter(
124124
const v8::FunctionCallbackInfo<v8::Value>& args);
125125
static void Close(const v8::FunctionCallbackInfo<v8::Value>& args);
126+
static void Dispose(const v8::FunctionCallbackInfo<v8::Value>& args);
126127
static void Prepare(const v8::FunctionCallbackInfo<v8::Value>& args);
127128
static void Exec(const v8::FunctionCallbackInfo<v8::Value>& args);
128129
static void CreateTagStore(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -266,6 +267,7 @@ class Session : public BaseObject {
266267
template <Sqlite3ChangesetGenFunc sqliteChangesetFunc>
267268
static void Changeset(const v8::FunctionCallbackInfo<v8::Value>& args);
268269
static void Close(const v8::FunctionCallbackInfo<v8::Value>& args);
270+
static void Dispose(const v8::FunctionCallbackInfo<v8::Value>& args);
269271
static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
270272
Environment* env);
271273
static BaseObjectPtr<Session> Create(Environment* env,

src/util.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,32 @@ void SetMethodNoSideEffect(Isolate* isolate,
598598
that->Set(name_string, t);
599599
}
600600

601+
void SetProtoDispose(v8::Isolate* isolate,
602+
v8::Local<v8::FunctionTemplate> that,
603+
v8::FunctionCallback callback) {
604+
Local<v8::Signature> signature = v8::Signature::New(isolate, that);
605+
Local<v8::FunctionTemplate> t =
606+
NewFunctionTemplate(isolate,
607+
callback,
608+
signature,
609+
v8::ConstructorBehavior::kThrow,
610+
v8::SideEffectType::kHasSideEffect);
611+
that->PrototypeTemplate()->Set(v8::Symbol::GetDispose(isolate), t);
612+
}
613+
614+
void SetProtoAsyncDispose(v8::Isolate* isolate,
615+
v8::Local<v8::FunctionTemplate> that,
616+
v8::FunctionCallback callback) {
617+
Local<v8::Signature> signature = v8::Signature::New(isolate, that);
618+
Local<v8::FunctionTemplate> t =
619+
NewFunctionTemplate(isolate,
620+
callback,
621+
signature,
622+
v8::ConstructorBehavior::kThrow,
623+
v8::SideEffectType::kHasSideEffect);
624+
that->PrototypeTemplate()->Set(v8::Symbol::GetAsyncDispose(isolate), t);
625+
}
626+
601627
void SetProtoMethod(v8::Isolate* isolate,
602628
Local<v8::FunctionTemplate> that,
603629
const std::string_view name,

src/util.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,16 @@ void SetMethodNoSideEffect(v8::Isolate* isolate,
932932
const std::string_view name,
933933
v8::FunctionCallback callback);
934934

935+
// Set the Symbol.dispose method on the prototype of the class.
936+
void SetProtoDispose(v8::Isolate* isolate,
937+
v8::Local<v8::FunctionTemplate> that,
938+
v8::FunctionCallback callback);
939+
940+
// Set the Symbol.asyncDispose method on the prototype of the class.
941+
void SetProtoAsyncDispose(v8::Isolate* isolate,
942+
v8::Local<v8::FunctionTemplate> that,
943+
v8::FunctionCallback callback);
944+
935945
enum class SetConstructorFunctionFlag {
936946
NONE,
937947
SET_CLASS_NAME,

test/parallel/test-sqlite-session.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,3 +540,18 @@ test('session.close() - closing twice', (t) => {
540540
message: 'session is not open'
541541
});
542542
});
543+
544+
test('session supports ERM', (t) => {
545+
const database = new DatabaseSync(':memory:');
546+
let afterDisposeSession;
547+
{
548+
using session = database.createSession();
549+
afterDisposeSession = session;
550+
const changeset = session.changeset();
551+
t.assert.ok(changeset instanceof Uint8Array);
552+
t.assert.strictEqual(changeset.length, 0);
553+
}
554+
t.assert.throws(() => afterDisposeSession.changeset(), {
555+
message: /session is not open/,
556+
});
557+
});

0 commit comments

Comments
 (0)