sqlite: reject reentry into a running statement by TrevorBurnham · Pull Request #65106 · nodejs/node · GitHub

Fixes: #65102

SQLite forbids stepping, resetting, or finalizing a statement while that statement's own user-defined function callback is on the stack. The callback depth added in 5cef767 (#64743) is tracked per database, so it cannot distinguish reentry into the running statement from the common pattern of querying a different statement from a callback.

This PR adds a per-statement flag, set for the duration of an execution, and rejects step/reset/finalize on that statement with ERR_INVALID_STATE: statement is currently being executed.

Severity

Reentry corrupts the running virtual machine. Resetting from a callback halts a VM whose sqlite3_step() frame is still live:

// SIGSEGV before this change
let stmt;
let first = true;
db.function('f', () => { if (first) { first = false; stmt.get(); } return 1; });
stmt = db.prepare('SELECT f(), x FROM t');
stmt.all();

Verified on v24.15.0 and current main. The crash needs the reentry to be bounded — the repros in #65102 recurse without limit, so V8's stack overflows before control returns into the corrupted step(), which is why that issue concluded it was correctness-only.

reentrant call from callback	before
get() / run() / all() (each resets)	SIGSEGV
iterator.return() (bare sqlite3_reset)	SIGSEGV
aggregate step / result callback	abort, CHECK(agg->initialized) in Release
iterator.next() (steps, no reset)	silently consumed rows from the iteration in progress
recursive get(), unbounded	Maximum call stack size exceeded
columns() / sourceSQL / expandedSQL	unaffected

The aggregate row is a distinct corruption path: the reentrant reset tears down aggregate state mid-xValue, tripping the CHECK in CustomAggregate::DestroyAggregateData. That CHECK is compiled into release builds, so it aborts in production rather than only under a debug build.

Covered entry points: all(), get(), run(), iterate(), iterator.next(), iterator.return(), close(), [Symbol.dispose](), and the four SQL tag store methods. close() and [Symbol.dispose]() are included because finalizing mid-step frees the virtual machine that sqlite3_step() is still executing.

Prior art: #63183 took this approach alongside its own database-level guard. That PR was closed once #64743 landed, so this salvages the per-statement half and builds on the guard already in main.

Review requested:

@nodejs/sqlite

Codecov Report

❌ Patch coverage is 61.53846% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.32%. Comparing base (e2d7b34) to head (abab573).
⚠️ Report is 62 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_sqlite.cc	54.54%	4 Missing and 6 partials ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #65106    +/-   ##
========================================
  Coverage   90.31%   90.32%            
========================================
  Files         759      760     +1     
  Lines      248290   248551   +261     
  Branches    46859    46918    +59     
========================================
+ Hits       224241   224501   +260     
+ Misses      15472    15460    -12     
- Partials     8577     8590    +13

Files with missing lines	Coverage Δ
src/node_sqlite.h	83.56% <100.00%> (+0.95%)	⬆️
src/node_sqlite.cc	81.24% <54.54%> (+0.18%)	⬆️

... and 69 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.

@trivikr I've addressed the issues you noted. The format-cpp check is now timing out after 15 minutes ("The operation was canceled"); the same check succeeds locally.

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

Commit Queue failed

- Loading data for nodejs/node/pull/65106
✔  Done loading data for nodejs/node/pull/65106
----------------------------------- PR info ------------------------------------
Title      sqlite: reject reentry into a running statement (#65106)
Author     Trevor Burnham <trevorburnham@gmail.com> (@TrevorBurnham)
Branch     TrevorBurnham:sqlite/guard-statement-reentry -> nodejs:main
Labels     c++, author ready, needs-ci, commit-queue, sqlite
Commits    1
 - sqlite: reject reentry into a running statement
Committers 1
 - Trevor Burnham <trevorburnham@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65106
Fixes: https://github.com/nodejs/node/issues/65102
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/65106
Fixes: https://github.com/nodejs/node/issues/65102
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 07 Aug 2026 14:58:01 GMT
   ✔  Approvals: 1
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/65106#pullrequestreview-4889248717
   ✘  This PR needs to wait 113 more hours to land (or 0 minutes if there is one more approval)
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-08-08T17:08:25Z: https://ci.nodejs.org/job/node-test-pull-request/75659/
- Querying data for job/node-test-pull-request/75659/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /Users/avivkeller/Documents/projects/nodejs/node/.ncu

/nodejs/node/actions/runs/

SQLite forbids stepping, resetting, or finalizing a statement while that statement's own user-defined function callback is on the stack. The callback depth added in 5cef767 is tracked per database, so it cannot tell reentry into the running statement apart from the common pattern of querying a different statement from a callback. Mark the statement being executed and reject step, reset, and finalize on that statement with ERR_INVALID_STATE. Statements other than the running one are unaffected. Reentry corrupts the running virtual machine rather than merely producing a wrong answer. Resetting from a callback halts a VM whose sqlite3_step() frame is still live, so a bounded reentrant get() or run(), and iterator.return() with its bare sqlite3_reset(), all segfault; from an aggregate's step or result callback the same reset tears down aggregate state mid-xValue and aborts on a CHECK in a release build. Where it does not crash it is still wrong: a reentrant iterator.next() silently consumed rows from the iteration in progress, and a recursive get() surfaced a V8 stack overflow instead of the constraint. close() and [Symbol.dispose]() are covered too, since finalizing mid-step frees the running VM. The mark is set before parameters are bound, so a getter or valueOf() that reenters while its own arguments are being evaluated is rejected as well. Without this, two iterators could share one virtual machine and interleave rows from a single result set. Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> Assisted-by: claude:opus-5

^ Updated the PR to cover sqlite3_reset. The PR description has been updated as well.

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

This PR was largely superseded by #65156. Closing it out. I've spun off one small follow-up PR: #65294

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem. labels Aug 7, 2026

This comment was marked as outdated.

Sign in to view

TrevorBurnham force-pushed the sqlite/guard-statement-reentry branch from ee4e9b5 to 986eb0e Compare August 7, 2026 17:41

trivikr requested changes Aug 8, 2026

View reviewed changes

Comment thread src/node_sqlite.cc

trivikr reviewed Aug 8, 2026

View reviewed changes

Comment thread test/parallel/test-sqlite-udf-statement-reentry.js

TrevorBurnham force-pushed the sqlite/guard-statement-reentry branch from 986eb0e to fde8f11 Compare August 8, 2026 12:09

trivikr reviewed Aug 8, 2026

View reviewed changes

Comment thread test/parallel/test-sqlite-udf-statement-reentry.js

Comment thread test/parallel/test-sqlite-udf-statement-reentry.js Outdated

TrevorBurnham force-pushed the sqlite/guard-statement-reentry branch from fde8f11 to 02e92f5 Compare August 8, 2026 14:58

trivikr approved these changes Aug 8, 2026

View reviewed changes

trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2026

TrevorBurnham mentioned this pull request Aug 8, 2026

stabilization of node:sqlite module #57445

Open

trivikr added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Aug 9, 2026

trivikr requested a review from Renegade334 August 9, 2026 07:15

trivikr added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 9, 2026

avivkeller added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 9, 2026

avivkeller added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 9, 2026

trivikr mentioned this pull request Aug 9, 2026

sqlite: prevent reentrant statement finalization from callbacks #64795

Closed

TrevorBurnham force-pushed the sqlite/guard-statement-reentry branch from 02e92f5 to abab573 Compare August 10, 2026 13:36

trivikr approved these changes Aug 10, 2026

View reviewed changes

trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 10, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 10, 2026

This comment was marked as resolved.

Sign in to view

panva removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 10, 2026

TrevorBurnham mentioned this pull request Aug 14, 2026

sqlite: reject reentry while binding parameters #65294

Merged

TrevorBurnham closed this Aug 14, 2026

sqlite: reject reentry into a running statement by TrevorBurnham · Pull Request #65106 · nodejs/node · GitHub

Uh oh!

Uh oh!

sqlite: reject reentry into a running statement - #65106

sqlite: reject reentry into a running statement#65106
TrevorBurnham wants to merge 1 commit into
nodejs:mainfrom
TrevorBurnham:sqlite/guard-statement-reentry

TrevorBurnham commented Aug 7, 2026 •
edited
Loading

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 7, 2026

Uh oh!

codecov Bot commented Aug 7, 2026 •
edited
Loading

Uh oh!

Uh oh!

This comment was marked as outdated.

TrevorBurnham commented Aug 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 8, 2026

Uh oh!

avivkeller commented Aug 9, 2026

Uh oh!

This comment was marked as resolved.

TrevorBurnham commented Aug 10, 2026

Uh oh!

nodejs-github-bot commented Aug 11, 2026

Uh oh!

TrevorBurnham commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sqlite: reject reentry into a running statement by TrevorBurnham · Pull Request #65106 · nodejs/node · GitHub

Uh oh!

Uh oh!

sqlite: reject reentry into a running statement - #65106

sqlite: reject reentry into a running statement#65106TrevorBurnham wants to merge 1 commit intonodejs:mainnodejs/node:mainfrom TrevorBurnham:sqlite/guard-statement-reentryTrevorBurnham/node:sqlite/guard-statement-reentryCopy head branch name to clipboard

Conversation

TrevorBurnham commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Severity

Uh oh!

nodejs-github-bot commented Aug 7, 2026

Uh oh!

codecov Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as outdated.

TrevorBurnham commented Aug 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 8, 2026

Uh oh!

avivkeller commented Aug 9, 2026

Uh oh!

This comment was marked as resolved.

TrevorBurnham commented Aug 10, 2026

Uh oh!

nodejs-github-bot commented Aug 11, 2026

Uh oh!

TrevorBurnham commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sqlite: reject reentry into a running statement#65106
TrevorBurnham wants to merge 1 commit into
nodejs:mainfrom
TrevorBurnham:sqlite/guard-statement-reentry

TrevorBurnham commented Aug 7, 2026 •
edited
Loading

codecov Bot commented Aug 7, 2026 •
edited
Loading