Repository navigation
sqlite: StatementSync#iterate fails with Error: statement has been finalized #57493
Description
Version
v23.10.0
Platform
- Microsoft Windows NT 10.0.26100.0 x64
- Microsoft Windows Server 2022 10.0.20348 (Github actions)
- Ubuntu 24.04.2 LTS (Github actions)
Subsystem
sqlite
What steps will reproduce the bug?
index.js
// = Setup =
import { DatabaseSync } from "node:sqlite";
const database = new DatabaseSync(":memory:");
database.exec(`
CREATE TABLE users (
user_id INTEGER PRIMARY KEY AUTOINCREMENT,
name TEXT NOT NULL UNIQUE
);
`);
for (let index = 0; index < 1e3; ++index) {
database.prepare(`INSERT INTO users (name) VALUES (:name);`).run({
name: Math.random().toString(36).slice(2),
});
}
// Double check all 1000 rows are inserted
console.log(database.prepare("SELECT count(*) as row_count FROM users;").get());
// = Actual buggy behaviour starts here =
// const iter = [...database.prepare('SELECT * from users;').iterate()][Symbol.iterator]();
const iter = database.prepare("SELECT * from users;").iterate();
for (const row of iter) {
console.log(row.user_id, row.name);
}node index.jsHow often does it reproduce? Is there a required condition?
Happens every time. Sometimes it errors, sometimes it just stops unexpectedly without an error.
It doesn't reproduce if you uncomment the line where I fetch all rows immediately by spreading it into an array and then turning it back into an iterator, so the issue comes from reading each value over time.
What is the expected behavior? Why is that the expected behavior?
It should not error and it should print 1000 rows.
What do you see instead?
90% of runs
293 27p5p7fqtms
294 pn2ij0uxxsq
[...]/sqlite-iterate.js:25
for (const row of iter) {
^
Error: statement has been finalized
at [...]/sqlite-iterate.js:25:12
at ModuleJob.run (node:internal/modules/esm/module_job:274:25)
at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:644:26)
at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:98:5) {
code: 'ERR_INVALID_STATE'
}
Node.js v23.10.0
OR
10% it just stops without an error. It should have printed up to 1000.
279 pjvk19yrl7h
280 xrj1kp6l93
281 e73tq3381ff
282 q93lik5apn
283 6kx6awjjuce
284 3h47eaan34j
285 rkrljlp5qnm
PS [...]\5d9342b0-468e-4b12-b854-28a5775d2fab>
Sometimes the values of the last "successful" row is garbage. Sometimes for name it prints undefined or something like ÙÙ¥Ñ88. The name should always match 0-9a-z so there is some sort of corruption, too.
Additional information
- Github actions log if it helps: https://github.com/melusc/node-bug-sqlite-iterate/actions/runs/13882797889
You can see on the Ubuntu run, it exits with an error. On the Windows run, it just exits for no apparent reason. That is random, though, not platform-dependent.
Activity
fix: Don't use `StatementSync#iterate` because it fails sometimes (no…
cjihrig commentedon Mar 16, 2025
cc: @tpoisseau
tpoisseau commentedon Mar 17, 2025
Hello,
Today
.iterate()is tested only with.toArray(). I assert iftoArrayworks, it's OK for iteration and iterators helpers. And it's tested on a few rows.I don't know when I will have time to work on this, v8 CPP is really hard for me. It look like, somewhere, the statement is closed / free during the iteration. I have no idea why.
fuchsia commentedon Mar 18, 2025
I think the problem is the iterator doesn't have a reference to the
StatementSyncobject returned bydatabase.prepare(). So as soon asdatabase.prepare("SELECT * from users;").iterate()has been called, the GC is free to collect the intermediateStatementSyncobject. And, when it does, the underlying sqlite statement is finalised.Proof: edit the code to this:
That seems to work till the cows come home, because I'm now hanging on to
stmtand so it can't be GC'd.I imagine the solution is to create a ref to the
StatementSyncobject in the iterator object returned byiterate()(i.e. from insideStatementSync::Iterate(const FunctionCallbackInfo<Value>&)) e.g. a[Symbol.stmt]property that links to it.cjihrig commentedon Mar 18, 2025
@fuchsia I believe your assessment is correct. The only thing I'd add is that the iterator can hold a reference to the
StatementSyncobject without the reference being exposed to users in JavaScript.fuchsia commentedon Mar 19, 2025
I expect to have some time next week when I could have a go at writing a fix. I've not built node before, and getting my system set up to do that is the most daunting bit! I've plenty of experience of C++ and using sqlite3 from C++, although it's a bit rusty.
sqlite: refactor prepared statement iterator
fox1t commentedon Mar 21, 2025
I just encountered this issue while working on https://github.com/ducktors/Valkeyrie
I have this test case that returns this error while I would expect a different one. Something like "Error: the db is closed', but it throws "Error: statement has been finalized" instead.
This is the test case:
Is this the expected behavior? Do I need to track the status of the database in the user land?
cjihrig commentedon Mar 21, 2025
@fox1t what you're seeing is not the same as this issue, which is specific to
iterate().You're using an additional API on top of
node:sqlite, but I think it's expected. In this code, I expectdbto close once it goes out of scope even though you've aliased it. Once the database is closed, all of the prepared statements associated with that database are finalized.If you try to use a database after it's closed, you get a message that the database is closed. If you try to use an individual prepared statement after the database is closed or the statement is otherwise finalized, you get the finalized error.
No, you shouldn't need to. There is also going to be a method for this in the next release (#57522).
fox1t commentedon Mar 21, 2025
Oh, @cjihrig, that's awesome. Thanks for sharing this info about the finalization of the statements. I didn't realize it until now. That's exactly what happens since I prepare all queries at the "driver" level. It is a tiny wrapper of the underlying backend (SQLite in this case) that doesn't add any logic. It's cool that isOpen is being added!
sqlite: refactor prepared statement iterator
sqlite: refactor prepared statement iterator
sqlite: refactor prepared statement iterator
sqlite: refactor prepared statement iterator
sqlite: refactor prepared statement iterator
sqlite: refactor prepared statement iterator
sqlite: refactor prepared statement iterator
sqlite: refactor prepared statement iterator