Skip to content

sqlite: do not leave database open after failed open#63854

Open
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:sqlite-fix-failed-open
Open

sqlite: do not leave database open after failed open#63854
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:sqlite-fix-failed-open

Conversation

@anonrig

@anonrig anonrig commented Jun 11, 2026

Copy link
Copy Markdown
Member

When sqlite3_open_v2() fails, SQLite still assigns a database handle in a "sick" state. Such a handle may only be used to retrieve error information and must then be released with sqlite3_close(). DatabaseSync::Open() kept the handle in connection_, so after a failed open():

  • database.isOpen incorrectly reported true.
  • Every method passed the "database is not open" check and called SQLite APIs on the sick handle, which is an API misuse.
  • database.function() and database.aggregate() leaked their user data in builds with SQLITE_ENABLE_API_ARMOR, because SQLite rejects calls on a sick handle with SQLITE_MISUSE before taking ownership of the user data. This is the leak reported by LeakSanitizer in the issue.
  • open() could not be retried because the database appeared to be open.

This change closes and resets the connection handle when open() fails at any point, so a failed open() leaves the database closed, subsequent method calls throw ERR_INVALID_STATE, and open() can be retried once the underlying problem is resolved. Error reporting is unchanged: the error message is read from the sick handle before it is closed.

Before (v24.16.0):

$ node -e "
const { DatabaseSync } = require('node:sqlite');
const db = new DatabaseSync('/nonexistent-dir/x.db', { open: false, readOnly: true });
try { db.open(); } catch {}
console.log(db.isOpen);      // true
db.function('f', () => {});  // operates on the sick handle
"

Fixes: #63831

When sqlite3_open_v2() fails, SQLite still assigns a database handle
in a "sick" state. Such a handle may only be used to retrieve the
error and must then be released with sqlite3_close(). Keeping it in
connection_ meant that after a failed open():

- isOpen incorrectly reported true,
- every method passed the "database is not open" check and called
  SQLite APIs on the sick handle, which is an API misuse,
- registering a user-defined function leaked its user data in builds
  with SQLITE_ENABLE_API_ARMOR, because SQLite rejects the call
  before taking ownership of the user data,
- open() could not be retried since the database appeared open.

Close and reset the connection handle when open() fails at any point
so that the database remains closed and open() can be retried.

Fixes: nodejs#63831
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@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 Jun 11, 2026

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 11, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 11, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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

Development

Successfully merging this pull request may close these issues.

sqlite: memory leak and sqlite API misuse

3 participants