sqlite: throw on disposal of an in-use session

Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
PR-URL: #65449
Fixes: #65447
Refs: #65349
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>

Author: araujogui

doc/api/sqlite.md

@@ -1023,7 +1023,9 @@ wrapper around [`sqlite3session_delete()`][].
 added: v24.9.0
 -->
 
-Closes the session. If the session is already closed, does nothing.
+Closes the session. If the session is already closed, then this is a no-op. An
+[`ERR_INVALID_STATE`][] error is thrown if the session is currently generating
+a changeset or patchset, under the same conditions as [`session.close()`][].
 
 ## Class: `StatementSync`
 
@@ -1915,6 +1917,7 @@ callback function to indicate what type of operation is being authorized.
 [`database.serialize()`]: #databaseserializedbname
 [`database.setAuthorizer()`]: #databasesetauthorizercallback
 [`diagnostics_channel`]: diagnostics_channel.md
+[`session.close()`]: #sessionclose
 [`sqlite3_backup_finish()`]: https://www.sqlite.org/c3ref/backup_finish.html#sqlite3backupfinish
 [`sqlite3_backup_init()`]: https://www.sqlite.org/c3ref/backup_finish.html#sqlite3backupinit
 [`sqlite3_backup_step()`]: https://www.sqlite.org/c3ref/backup_finish.html#sqlite3backupstep

src/node_sqlite.cc

@@ -4350,12 +4350,17 @@ void Session::Close(const FunctionCallbackInfo<Value>& args) {
   session->Delete();
 }
 
-void Session::Dispose(const v8::FunctionCallbackInfo<v8::Value>& args) {
-  v8::TryCatch try_catch(args.GetIsolate());
-  Close(args);
-  if (try_catch.HasCaught()) {
-    CHECK(try_catch.CanContinue());
+void Session::Dispose(const FunctionCallbackInfo<Value>& args) {
+  Session* session;
+  ASSIGN_OR_RETURN_UNWRAP(&session, args.This());
+  Environment* env = Environment::GetCurrent(args);
+  if (session->session_ == nullptr) {
+    return;
   }
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, session->is_generating_changeset_, "session is currently in use");
+
+  session->Delete();
 }
 
 void Session::Delete() {

test/parallel/test-sqlite-session.js

@@ -612,28 +612,46 @@ test('session.close() - closing twice', (t) => {
   });
 });
 
-test('session.close() - while generating changes throws exception', (t) => {
-  for (const method of ['changeset', 'patchset']) {
-    const database = new DatabaseSync(':memory:');
-    database.exec('CREATE TABLE data(key INTEGER PRIMARY KEY, value TEXT)');
-
-    const session = database.createSession({ table: 'data' });
-    database.exec("INSERT INTO data VALUES (1, 'a'), (2, 'b'), (3, 'c')");
-    database.setAuthorizer(() => {
-      session.close();
-      return constants.SQLITE_OK;
-    });
+test('session close and dispose - while generating changes throws exception', (t) => {
+  for (const close of ['close', Symbol.dispose]) {
+    for (const method of ['changeset', 'patchset']) {
+      const database = new DatabaseSync(':memory:');
+      database.exec('CREATE TABLE data(key INTEGER PRIMARY KEY, value TEXT)');
+
+      const session = database.createSession({ table: 'data' });
+      database.exec("INSERT INTO data VALUES (1, 'a'), (2, 'b'), (3, 'c')");
+      database.setAuthorizer(() => {
+        session[close]();
+        return constants.SQLITE_OK;
+      });
 
-    t.assert.throws(() => session[method](), {
-      code: 'ERR_INVALID_STATE',
-      message: 'session is currently in use',
-    });
+      t.assert.throws(() => session[method](), {
+        code: 'ERR_INVALID_STATE',
+        message: 'session is currently in use',
+      });
 
-    database.setAuthorizer(null);
-    t.assert.notStrictEqual(session[method]().length, 0);
+      database.setAuthorizer(null);
+      t.assert.notStrictEqual(session[method]().length, 0);
+    }
   }
 });
 
+test('session[Symbol.dispose]() - closed session is a no-op', () => {
+  const database = new DatabaseSync(':memory:');
+  const session = database.createSession();
+  session.close();
+
+  session[Symbol.dispose]();
+});
+
+test('session[Symbol.dispose]() - after closing database is a no-op', () => {
+  const database = new DatabaseSync(':memory:');
+  const session = database.createSession();
+  database.close();
+
+  session[Symbol.dispose]();
+});
+
 test('session - keeps its database alive after the db handle is dropped', async (t) => {
   const { gcUntil, onGC } = require('../common/gc');