sqlite: reject closing a session from a callback

[TrevorBurnham]

session.close() from a SQLite callback frees the session while SQLite is still using it:

constdb=newDatabaseSync(':memory:');db.exec('CREATE TABLE t(a PRIMARY KEY)');constsession=db.createSession();db.setAuthorizer(()=>(session.close(),constants.SQLITE_OK));db.exec('INSERT INTO t VALUES (1)');// segfault

SQLite's pre-update hook walks the connection's session list and calls sessionTableInfo(), which prepares and steps PRAGMA table_xinfo. That reaches JavaScript, so a callback runs while the walk still holds pointers into the session it is visiting; sqlite3session_delete() there frees them.

Two callbacks reach that window: an authorizer callback (the reported case) and a 'sqlite.db.query' subscriber, which fires on SQLITE_TRACE_PROFILE when the internal PRAGMA finishes. Both segfault on main, so an authorizer-only guard would be incomplete.

The guard rejects close() and Symbol.dispose whenever the connection is in any callback, matching the existing db.close() rule. Node cannot tell whether SQLite is currently inside xPreUpdate, so a narrower guard would rest on no other callback ever running there. It does newly reject closing a session from a user-defined function, which is safe today. Disposal stays a no-op for an already-closed session; only a live one throws.

Rebased onto #65449, which made session[Symbol.dispose]() throw for a session that is generating a changeset. This guard follows that precedent: the callback check sits below the in-use check, so the more specific message still wins, and #65449's test covers both orders. One consequence is that a using declaration inside a callback demotes the block's own error to SuppressedError; test-sqlite-session.js pins that.

Fixes: #65428

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (0544741) to head (2e65c4c).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65454 +/- ##
==========================================
- Coverage 90.07% 90.06% -0.02% 
==========================================
Files 751 751 Lines 254921 254923 +2 Branches 48129 48124 -5 ==========================================
- Hits 229627 229602 -25 - Misses 16479 16504 +25 - Partials 8815 8817 +2 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	82.12% <100.00%> (+0.01%)	⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/76739/

[trivikr]

Landed in 01c1300

[trivikr]

Landed manually, since the CI failures in test-module-builtin-experimental are unrelated to this PR and were fixed in #65636.

[TrevorBurnham]

session.close() from a SQLite callback frees the session while SQLite is still using it:

constdb=newDatabaseSync(':memory:');db.exec('CREATE TABLE t(a PRIMARY KEY)');constsession=db.createSession();db.setAuthorizer(()=>(session.close(),constants.SQLITE_OK));db.exec('INSERT INTO t VALUES (1)');// segfault

SQLite's pre-update hook walks the connection's session list and calls sessionTableInfo(), which prepares and steps PRAGMA table_xinfo. That reaches JavaScript, so a callback runs while the walk still holds pointers into the session it is visiting; sqlite3session_delete() there frees them.

Two callbacks reach that window: an authorizer callback (the reported case) and a 'sqlite.db.query' subscriber, which fires on SQLITE_TRACE_PROFILE when the internal PRAGMA finishes. Both segfault on main, so an authorizer-only guard would be incomplete.

The guard rejects close() and Symbol.dispose whenever the connection is in any callback, matching the existing db.close() rule. Node cannot tell whether SQLite is currently inside xPreUpdate, so a narrower guard would rest on no other callback ever running there. It does newly reject closing a session from a user-defined function, which is safe today. Disposal stays a no-op for an already-closed session; only a live one throws.

Rebased onto #65449, which made session[Symbol.dispose]() throw for a session that is generating a changeset. This guard follows that precedent: the callback check sits below the in-use check, so the more specific message still wins, and #65449's test covers both orders. One consequence is that a using declaration inside a callback demotes the block's own error to SuppressedError; test-sqlite-session.js pins that.

Fixes: #65428

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (0544741) to head (2e65c4c).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65454 +/- ##
==========================================
- Coverage 90.07% 90.06% -0.02% 
==========================================
Files 751 751 Lines 254921 254923 +2 Branches 48129 48124 -5 ==========================================
- Hits 229627 229602 -25 - Misses 16479 16504 +25 - Partials 8815 8817 +2 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	82.12% <100.00%> (+0.01%)	⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/76739/

[trivikr]

Landed in 01c1300

[trivikr]

Landed manually, since the CI failures in test-module-builtin-experimental are unrelated to this PR and were fixed in #65636.

[TrevorBurnham]

session.close() from a SQLite callback frees the session while SQLite is still using it:

constdb=newDatabaseSync(':memory:');db.exec('CREATE TABLE t(a PRIMARY KEY)');constsession=db.createSession();db.setAuthorizer(()=>(session.close(),constants.SQLITE_OK));db.exec('INSERT INTO t VALUES (1)');// segfault

SQLite's pre-update hook walks the connection's session list and calls sessionTableInfo(), which prepares and steps PRAGMA table_xinfo. That reaches JavaScript, so a callback runs while the walk still holds pointers into the session it is visiting; sqlite3session_delete() there frees them.

Two callbacks reach that window: an authorizer callback (the reported case) and a 'sqlite.db.query' subscriber, which fires on SQLITE_TRACE_PROFILE when the internal PRAGMA finishes. Both segfault on main, so an authorizer-only guard would be incomplete.

The guard rejects close() and Symbol.dispose whenever the connection is in any callback, matching the existing db.close() rule. Node cannot tell whether SQLite is currently inside xPreUpdate, so a narrower guard would rest on no other callback ever running there. It does newly reject closing a session from a user-defined function, which is safe today. Disposal stays a no-op for an already-closed session; only a live one throws.

Rebased onto #65449, which made session[Symbol.dispose]() throw for a session that is generating a changeset. This guard follows that precedent: the callback check sits below the in-use check, so the more specific message still wins, and #65449's test covers both orders. One consequence is that a using declaration inside a callback demotes the block's own error to SuppressedError; test-sqlite-session.js pins that.

Fixes: #65428

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (0544741) to head (2e65c4c).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65454 +/- ##
==========================================
- Coverage 90.07% 90.06% -0.02% 
==========================================
Files 751 751 Lines 254921 254923 +2 Branches 48129 48124 -5 ==========================================
- Hits 229627 229602 -25 - Misses 16479 16504 +25 - Partials 8815 8817 +2 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	82.12% <100.00%> (+0.01%)	⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/76739/

[trivikr]

Landed in 01c1300

[trivikr]

Landed manually, since the CI failures in test-module-builtin-experimental are unrelated to this PR and were fixed in #65636.

[TrevorBurnham]

session.close() from a SQLite callback frees the session while SQLite is still using it:

constdb=newDatabaseSync(':memory:');db.exec('CREATE TABLE t(a PRIMARY KEY)');constsession=db.createSession();db.setAuthorizer(()=>(session.close(),constants.SQLITE_OK));db.exec('INSERT INTO t VALUES (1)');// segfault

SQLite's pre-update hook walks the connection's session list and calls sessionTableInfo(), which prepares and steps PRAGMA table_xinfo. That reaches JavaScript, so a callback runs while the walk still holds pointers into the session it is visiting; sqlite3session_delete() there frees them.

Two callbacks reach that window: an authorizer callback (the reported case) and a 'sqlite.db.query' subscriber, which fires on SQLITE_TRACE_PROFILE when the internal PRAGMA finishes. Both segfault on main, so an authorizer-only guard would be incomplete.

The guard rejects close() and Symbol.dispose whenever the connection is in any callback, matching the existing db.close() rule. Node cannot tell whether SQLite is currently inside xPreUpdate, so a narrower guard would rest on no other callback ever running there. It does newly reject closing a session from a user-defined function, which is safe today. Disposal stays a no-op for an already-closed session; only a live one throws.

Rebased onto #65449, which made session[Symbol.dispose]() throw for a session that is generating a changeset. This guard follows that precedent: the callback check sits below the in-use check, so the more specific message still wins, and #65449's test covers both orders. One consequence is that a using declaration inside a callback demotes the block's own error to SuppressedError; test-sqlite-session.js pins that.

Fixes: #65428

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (0544741) to head (2e65c4c).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65454 +/- ##
==========================================
- Coverage 90.07% 90.06% -0.02% 
==========================================
Files 751 751 Lines 254921 254923 +2 Branches 48129 48124 -5 ==========================================
- Hits 229627 229602 -25 - Misses 16479 16504 +25 - Partials 8815 8817 +2 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	82.12% <100.00%> (+0.01%)	⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/76739/

[trivikr]

Landed in 01c1300

[trivikr]

Landed manually, since the CI failures in test-module-builtin-experimental are unrelated to this PR and were fixed in #65636.

[TrevorBurnham]

session.close() from a SQLite callback frees the session while SQLite is still using it:

constdb=newDatabaseSync(':memory:');db.exec('CREATE TABLE t(a PRIMARY KEY)');constsession=db.createSession();db.setAuthorizer(()=>(session.close(),constants.SQLITE_OK));db.exec('INSERT INTO t VALUES (1)');// segfault

SQLite's pre-update hook walks the connection's session list and calls sessionTableInfo(), which prepares and steps PRAGMA table_xinfo. That reaches JavaScript, so a callback runs while the walk still holds pointers into the session it is visiting; sqlite3session_delete() there frees them.

Two callbacks reach that window: an authorizer callback (the reported case) and a 'sqlite.db.query' subscriber, which fires on SQLITE_TRACE_PROFILE when the internal PRAGMA finishes. Both segfault on main, so an authorizer-only guard would be incomplete.

The guard rejects close() and Symbol.dispose whenever the connection is in any callback, matching the existing db.close() rule. Node cannot tell whether SQLite is currently inside xPreUpdate, so a narrower guard would rest on no other callback ever running there. It does newly reject closing a session from a user-defined function, which is safe today. Disposal stays a no-op for an already-closed session; only a live one throws.

Rebased onto #65449, which made session[Symbol.dispose]() throw for a session that is generating a changeset. This guard follows that precedent: the callback check sits below the in-use check, so the more specific message still wins, and #65449's test covers both orders. One consequence is that a using declaration inside a callback demotes the block's own error to SuppressedError; test-sqlite-session.js pins that.

Fixes: #65428

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (0544741) to head (2e65c4c).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65454 +/- ##
==========================================
- Coverage 90.07% 90.06% -0.02% 
==========================================
Files 751 751 Lines 254921 254923 +2 Branches 48129 48124 -5 ==========================================
- Hits 229627 229602 -25 - Misses 16479 16504 +25 - Partials 8815 8817 +2 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	82.12% <100.00%> (+0.01%)	⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/76739/

[trivikr]

Landed in 01c1300

[trivikr]

Landed manually, since the CI failures in test-module-builtin-experimental are unrelated to this PR and were fixed in #65636.

[TrevorBurnham]

session.close() from a SQLite callback frees the session while SQLite is still using it:

constdb=newDatabaseSync(':memory:');db.exec('CREATE TABLE t(a PRIMARY KEY)');constsession=db.createSession();db.setAuthorizer(()=>(session.close(),constants.SQLITE_OK));db.exec('INSERT INTO t VALUES (1)');// segfault

SQLite's pre-update hook walks the connection's session list and calls sessionTableInfo(), which prepares and steps PRAGMA table_xinfo. That reaches JavaScript, so a callback runs while the walk still holds pointers into the session it is visiting; sqlite3session_delete() there frees them.

Two callbacks reach that window: an authorizer callback (the reported case) and a 'sqlite.db.query' subscriber, which fires on SQLITE_TRACE_PROFILE when the internal PRAGMA finishes. Both segfault on main, so an authorizer-only guard would be incomplete.

The guard rejects close() and Symbol.dispose whenever the connection is in any callback, matching the existing db.close() rule. Node cannot tell whether SQLite is currently inside xPreUpdate, so a narrower guard would rest on no other callback ever running there. It does newly reject closing a session from a user-defined function, which is safe today. Disposal stays a no-op for an already-closed session; only a live one throws.

Rebased onto #65449, which made session[Symbol.dispose]() throw for a session that is generating a changeset. This guard follows that precedent: the callback check sits below the in-use check, so the more specific message still wins, and #65449's test covers both orders. One consequence is that a using declaration inside a callback demotes the block's own error to SuppressedError; test-sqlite-session.js pins that.

Fixes: #65428

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (0544741) to head (2e65c4c).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65454 +/- ##
==========================================
- Coverage 90.07% 90.06% -0.02% 
==========================================
Files 751 751 Lines 254921 254923 +2 Branches 48129 48124 -5 ==========================================
- Hits 229627 229602 -25 - Misses 16479 16504 +25 - Partials 8815 8817 +2 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	82.12% <100.00%> (+0.01%)	⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/76739/

[trivikr]

Landed in 01c1300

[trivikr]

Landed manually, since the CI failures in test-module-builtin-experimental are unrelated to this PR and were fixed in #65636.

[TrevorBurnham]

session.close() from a SQLite callback frees the session while SQLite is still using it:

constdb=newDatabaseSync(':memory:');db.exec('CREATE TABLE t(a PRIMARY KEY)');constsession=db.createSession();db.setAuthorizer(()=>(session.close(),constants.SQLITE_OK));db.exec('INSERT INTO t VALUES (1)');// segfault

SQLite's pre-update hook walks the connection's session list and calls sessionTableInfo(), which prepares and steps PRAGMA table_xinfo. That reaches JavaScript, so a callback runs while the walk still holds pointers into the session it is visiting; sqlite3session_delete() there frees them.

Two callbacks reach that window: an authorizer callback (the reported case) and a 'sqlite.db.query' subscriber, which fires on SQLITE_TRACE_PROFILE when the internal PRAGMA finishes. Both segfault on main, so an authorizer-only guard would be incomplete.

The guard rejects close() and Symbol.dispose whenever the connection is in any callback, matching the existing db.close() rule. Node cannot tell whether SQLite is currently inside xPreUpdate, so a narrower guard would rest on no other callback ever running there. It does newly reject closing a session from a user-defined function, which is safe today. Disposal stays a no-op for an already-closed session; only a live one throws.

Rebased onto #65449, which made session[Symbol.dispose]() throw for a session that is generating a changeset. This guard follows that precedent: the callback check sits below the in-use check, so the more specific message still wins, and #65449's test covers both orders. One consequence is that a using declaration inside a callback demotes the block's own error to SuppressedError; test-sqlite-session.js pins that.

Fixes: #65428

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (0544741) to head (2e65c4c).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65454 +/- ##
==========================================
- Coverage 90.07% 90.06% -0.02% 
==========================================
Files 751 751 Lines 254921 254923 +2 Branches 48129 48124 -5 ==========================================
- Hits 229627 229602 -25 - Misses 16479 16504 +25 - Partials 8815 8817 +2 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	82.12% <100.00%> (+0.01%)	⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/76739/

[trivikr]

Landed in 01c1300

[trivikr]

Landed manually, since the CI failures in test-module-builtin-experimental are unrelated to this PR and were fixed in #65636.

[TrevorBurnham]

session.close() from a SQLite callback frees the session while SQLite is still using it:

constdb=newDatabaseSync(':memory:');db.exec('CREATE TABLE t(a PRIMARY KEY)');constsession=db.createSession();db.setAuthorizer(()=>(session.close(),constants.SQLITE_OK));db.exec('INSERT INTO t VALUES (1)');// segfault

SQLite's pre-update hook walks the connection's session list and calls sessionTableInfo(), which prepares and steps PRAGMA table_xinfo. That reaches JavaScript, so a callback runs while the walk still holds pointers into the session it is visiting; sqlite3session_delete() there frees them.

Two callbacks reach that window: an authorizer callback (the reported case) and a 'sqlite.db.query' subscriber, which fires on SQLITE_TRACE_PROFILE when the internal PRAGMA finishes. Both segfault on main, so an authorizer-only guard would be incomplete.

The guard rejects close() and Symbol.dispose whenever the connection is in any callback, matching the existing db.close() rule. Node cannot tell whether SQLite is currently inside xPreUpdate, so a narrower guard would rest on no other callback ever running there. It does newly reject closing a session from a user-defined function, which is safe today. Disposal stays a no-op for an already-closed session; only a live one throws.

Rebased onto #65449, which made session[Symbol.dispose]() throw for a session that is generating a changeset. This guard follows that precedent: the callback check sits below the in-use check, so the more specific message still wins, and #65449's test covers both orders. One consequence is that a using declaration inside a callback demotes the block's own error to SuppressedError; test-sqlite-session.js pins that.

Fixes: #65428

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (0544741) to head (2e65c4c).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65454 +/- ##
==========================================
- Coverage 90.07% 90.06% -0.02% 
==========================================
Files 751 751 Lines 254921 254923 +2 Branches 48129 48124 -5 ==========================================
- Hits 229627 229602 -25 - Misses 16479 16504 +25 - Partials 8815 8817 +2 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	82.12% <100.00%> (+0.01%)	⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/76739/

[trivikr]

Landed in 01c1300

[trivikr]

Landed manually, since the CI failures in test-module-builtin-experimental are unrelated to this PR and were fixed in #65636.