Skip to content

Commit 6184852

Browse files
committed
fixup! sqlite: reject closing a session from a callback
Pin the deliberate over-rejection with a test: closing from a user-defined function is safe today but rejected anyway, because Node cannot tell whether SQLite is inside its pre-update hook. Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
1 parent 10ebc04 commit 6184852

1 file changed

Lines changed: 28 additions & 0 deletions

File tree

‎test/parallel/test-sqlite-session.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,34 @@ suite('session.close() - from a callback', () => {
705705
t.assert.notStrictEqual(session.changeset().length, 0);
706706
session.close();
707707
});
708+
709+
// Deliberately broader than the crash: the pre-update hook is not on the
710+
// stack here, so this close is safe today. Node cannot tell whether SQLite
711+
// is inside that hook, so every callback is rejected. This pins the
712+
// trade-off rather than leaving it to be discovered as a regression.
713+
it(`rejects ${method} from a user-defined function`, (t) => {
714+
const database = new DatabaseSync(':memory:');
715+
database.exec('CREATE TABLE data(key INTEGER PRIMARY KEY)');
716+
const session = database.createSession();
717+
let outcome = 'callback did not run';
718+
719+
database.function('f', (x) => {
720+
try {
721+
closeSession(session);
722+
outcome = 'did not throw';
723+
} catch (err) {
724+
outcome = `${err.code}: ${err.message}`;
725+
}
726+
return x;
727+
});
728+
729+
database.exec('SELECT f(1)');
730+
t.assert.strictEqual(outcome, expectedError);
731+
732+
// Still closable once the callback is off the stack.
733+
session.close();
734+
t.assert.throws(() => session.close(), { message: 'session is not open' });
735+
});
708736
}
709737

710738
it('leaves an already closed session disposable from a callback', (t) => {

0 commit comments

Comments
 (0)