sqlite: reject reentry while binding parameters by TrevorBurnham · Pull Request #65294 · nodejs/node · GitHub

Follow-up for #65156. That PR added tracking for the statements currently being stepped and rejects reentry into them. The guard is established inside the execution helpers, though, which the entry points reach only after resetting the statement and binding its parameters.

Binding reads properties off the supplied object, so a named-parameter getter runs JavaScript in that window. On current main:

const { DatabaseSync } = require('node:sqlite');
const db = new DatabaseSync(':memory:');
db.exec('CREATE TABLE data (value INTEGER); INSERT INTO data VALUES (1),(2),(3);');
const st = db.prepare('SELECT value FROM data WHERE value >= $min');

let inner;
const params = { get $min() { inner ??= st.iterate({ $min: 1 }); return 1; } };

const outer = st.iterate(params);
console.log(outer.next(), inner.next(), outer.next(), inner.next());

{ done: false, value: { value: 1 } }   // outer
{ done: false, value: { value: 2 } }   // inner
{ done: false, value: { value: 3 } }   // outer
{ done: true,  value: null }           // inner

Two live iterators over one virtual machine. The reset-generation check does not catch it because the inner iterate() bumps the generation before the outer iterator is constructed, so both record the same value and neither is invalidated.

This PR hoists the guard to the four StatementSync entry points, before the reset, so it spans binding as well as stepping. The guards inside the helpers are left alone; stepping_statements_ is a stack, so the nested acquisition is balanced. It also adds test cases salvaged from #65106, which was superseded by #65156.

c4b755e tracks the statements currently being stepped and rejects reentry into them, but the guard is established inside the execution helpers, after the entry point has already reset the statement and bound its parameters. Binding reads properties off the supplied object, so a named-parameter getter runs JavaScript in that window. Reentering the same statement there resets it a second time and, for iterate(), hands out a second iterator; both iterators record the same reset generation, so neither is invalidated and they interleave rows from one virtual machine. Establish the guard at the four StatementSync entry points instead, before the reset, so it spans binding as well as stepping. The existing guards inside the helpers are left in place; the stepping set is a stack, so the nested acquisition is balanced. Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> Assisted-by: claude:opus-5

Review requested:

@nodejs/sqlite

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.30%. Comparing base (c8996ea) to head (58b955d).
⚠️ Report is 99 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65294      +/-   ##
==========================================
- Coverage   90.33%   90.30%   -0.03%     
==========================================
  Files         751      751              
  Lines      250048   250239     +191     
  Branches    47254    47300      +46     
==========================================
+ Hits       225877   225990     +113     
- Misses      15553    15620      +67     
- Partials     8618     8629      +11

Files with missing lines	Coverage Δ
src/node_sqlite.cc	82.20% <100.00%> (-0.05%)	⬇️

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

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

Landed in 5fc9f1f

c4b755e tracks the statements currently being stepped and rejects reentry into them, but the guard is established inside the execution helpers, after the entry point has already reset the statement and bound its parameters. Binding reads properties off the supplied object, so a named-parameter getter runs JavaScript in that window. Reentering the same statement there resets it a second time and, for iterate(), hands out a second iterator; both iterators record the same reset generation, so neither is invalidated and they interleave rows from one virtual machine. Establish the guard at the four StatementSync entry points instead, before the reset, so it spans binding as well as stepping. The existing guards inside the helpers are left in place; the stepping set is a stack, so the nested acquisition is balanced. Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> Assisted-by: claude:opus-5 PR-URL: #65294 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

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 14, 2026

TrevorBurnham mentioned this pull request Aug 14, 2026

sqlite: reject reentry into a running statement #65106

Closed

trivikr approved these changes Aug 19, 2026

View reviewed changes

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

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

This comment was marked as outdated.

Sign in to view

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

trivikr requested a review from geeksilva97 August 20, 2026 22:41

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

nodejs-github-bot merged commit 5fc9f1f into nodejs:main Aug 21, 2026
88 checks passed

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 21, 2026

github-actions Bot mentioned this pull request Aug 25, 2026

2026-08-26, Version 26.8.0 (Current) #65551

Open

sqlite: reject reentry while binding parameters by TrevorBurnham · Pull Request #65294 · nodejs/node · GitHub

Uh oh!

Uh oh!

sqlite: reject reentry while binding parameters - #65294

sqlite: reject reentry while binding parameters#65294
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
TrevorBurnham:sqlite/binding-window-reentry

TrevorBurnham commented Aug 14, 2026 •
edited
Loading

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 14, 2026

Uh oh!

codecov Bot commented Aug 15, 2026 •
edited
Loading

Uh oh!

Uh oh!

This comment was marked as outdated.

nodejs-github-bot commented Aug 19, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sqlite: reject reentry while binding parameters by TrevorBurnham · Pull Request #65294 · nodejs/node · GitHub

Uh oh!

Uh oh!

sqlite: reject reentry while binding parameters - #65294

sqlite: reject reentry while binding parameters#65294nodejs-github-bot merged 1 commit intonodejs:mainnodejs/node:mainfrom TrevorBurnham:sqlite/binding-window-reentryTrevorBurnham/node:sqlite/binding-window-reentryCopy head branch name to clipboard

Conversation

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

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 14, 2026

Uh oh!

codecov Bot commented Aug 15, 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.

nodejs-github-bot commented Aug 19, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sqlite: reject reentry while binding parameters#65294
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
TrevorBurnham:sqlite/binding-window-reentry

TrevorBurnham commented Aug 14, 2026 •
edited
Loading

codecov Bot commented Aug 15, 2026 •
edited
Loading