Skip to content

sqlite: check database state before calling SQLite - #64812

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:sqlite-crash-api-after-close
Open

sqlite: check database state before calling SQLite#64812
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:sqlite-crash-api-after-close

Conversation

@trivikr

@trivikr trivikr commented Jul 29, 2026

Copy link
Copy Markdown
Member

Fixes: #64811

This change checks that the database is open before calling the corresponding
SQLite APIs. Both methods now throw ERR_INVALID_STATE, consistent with other
DatabaseSync methods.


Assisted-by: codex:gpt-5.6-sol

@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 Jul 29, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (6d0db4a) to head (76df88c).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64812      +/-   ##
==========================================
- Coverage   90.15%   90.14%   -0.01%     
==========================================
  Files         746      746              
  Lines      242660   242821     +161     
  Branches    45716    45765      +49     
==========================================
+ Hits       218763   218896     +133     
- Misses      15399    15419      +20     
- Partials     8498     8506       +8     
Files with missing lines Coverage Δ
src/node_sqlite.cc 80.88% <60.00%> (+0.21%) ⬆️

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

Comment thread src/node_sqlite.cc Outdated
@geeksilva97 geeksilva97 added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 29, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 29, 2026
@nodejs-github-bot

This comment was marked as outdated.

Check whether the database is open in enableLoadExtension() and
setAuthorizer() before passing the connection to SQLite. This prevents
calls after close() from terminating the process and makes them throw
ERR_INVALID_STATE instead.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
@trivikr
trivikr force-pushed the sqlite-crash-api-after-close branch from bff0854 to 76df88c Compare July 29, 2026 13:57
@trivikr
trivikr requested a review from Renegade334 July 29, 2026 13:58
@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jul 29, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 29, 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 crashes when enableLoadExtension() or setAuthorizer() is called after close()

4 participants