GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — sqlite: reject reentry while binding parameters by TrevorBurnham · Pull Request #65294 · nodejs/node

Conversation

@TrevorBurnham

@TrevorBurnham TrevorBurnham commented Aug 14, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@TrevorBurnham
sqlite: reject reentry while binding parameters
58b955d 
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 <[email protected]>
Assisted-by: claude:opus-5
@nodejs-github-bot

nodejs-github-bot commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot 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 TrevorBurnham mentioned this pull request Aug 14, 2026
sqlite: reject reentry into a running statement #65106
Closed
@codecov

codecov Bot commented Aug 15, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
trivikr
trivikr approved these changes Aug 19, 2026
View reviewed changes
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 19, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 19, 2026
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

nodejs-github-bot commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@trivikr trivikr added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Aug 20, 2026
@trivikr
trivikr requested a review from geeksilva97 August 20, 2026 22:41
@trivikr trivikr added the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 21, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 5fc9f1f into nodejs:main Aug 21, 2026
88 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Landed in 5fc9f1f
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 21, 2026
aduh95 pushed a commit that referenced this pull request Aug 25, 2026
@TrevorBurnham @aduh95
sqlite: reject reentry while binding parameters
d88f468 
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 <[email protected]>
Assisted-by: claude:opus-5
PR-URL: #65294
Reviewed-By: Trivikram Kamat <[email protected]>
aduh95 pushed a commit that referenced this pull request Aug 25, 2026
@TrevorBurnham @aduh95
sqlite: reject reentry while binding parameters
6ec735c 
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 <[email protected]>
Assisted-by: claude:opus-5
PR-URL: #65294
Reviewed-By: Trivikram Kamat <[email protected]>
@github-actions github-actions Bot mentioned this pull request Aug 25, 2026
2026-08-26, Version 26.8.0 (Current) #65551
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@trivikr trivikr trivikr approved these changes
@geeksilva97 geeksilva97 Awaiting requested review from geeksilva97

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. 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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@TrevorBurnham @nodejs-github-bot @trivikr
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.