## Summary
- track sessions created by `DatabaseSync`
- close and invalidate active sessions before closing the database
connection
- keep sessions from an earlier connection closed after the database is
reopened
## Motivation
Session objects checked whether the database's shared connection state
was populated. Reopening the database populated that state again, so
sessions created for the previous connection could be treated as open.
Tying each session handle to the database's active-session list keeps
its lifecycle associated with the connection that created it.
## Testing
- `cargo check -p deno --lib`
- `cargo test unit_node::sqlite_test`
- `./tools/format.js ext/node_sqlite/database.rs
ext/node_sqlite/session.rs tests/unit_node/sqlite_test.ts`
- `./tools/lint.js`
This prevents `DatabaseSync.deserialize()` from replacing database
contents while a user-defined SQLite callback is active.
`deserialize()` finalizes existing statements, so calling it reentrantly
could invalidate the statement SQLite is currently executing. The method
now uses the existing callback-depth guard and returns
`ERR_INVALID_STATE`, matching `DatabaseSync.close()` behavior.
Validation:
- `./tools/format.js`
- `cargo build --bin deno`
- focused `DatabaseSync.deserialize` callback regression
- full `tests/unit_node/sqlite_test.ts` suite (78 tests)
This makes `node:sqlite` treat JavaScript conversion failures as normal,
catchable errors.
V8 property and index access can execute JavaScript and fail, while C
string conversion rejects embedded null bytes. The affected statement
binding, tag store, function and aggregate option, iterator setup, and
session option paths now propagate those failures instead of assuming
conversion always succeeds. Result and error properties are also defined
directly to avoid invoking inherited setters.
After a failed conversion, the database remains usable by subsequent
operations.
`node:sqlite` disables `ATTACH DATABASE` when a process does not have
full permissions for the database path, because an attached database can
reach file paths outside the original database path. It does this by capping
`SQLITE_LIMIT_ATTACHED` at 0 for those connections.
The `DatabaseSync` limits support could raise that cap back up, which
undoes the protection. `apply_initial_limits` ran after the cap was set, so a
`new DatabaseSync(path, { limits: { attach: 1 } })` option re-enabled
attach, and the `db.limits.attach` setter wrote the limit directly with no
guard. In both cases a process with only scoped read/write access to one directory
could then `ATTACH DATABASE` a file in a directory it was explicitly denied.
This treats the `SQLITE_LIMIT_ATTACHED = 0` cap as authoritative for
those connections rather than user-tunable. Both surfaces now reject raising
the cap the same way: the `limits.attach` constructor option and the
`db.limits.attach` setter throw `ERR_ACCESS_DENIED` when asked to raise the limit above 0
while the cap is in force (a value of 0, keeping it disabled, is still accepted).
The setter error is a plain `Error`, not a `TypeError`, since an
access-denied condition is not a type error. When the process does have full
permissions nothing changes: the constructor option and the setter raise the limit
as before.
Added a spec test that runs with scoped `--allow-read`/`--allow-write`
on one directory and explicit `--deny-read`/`--deny-write` on another, and
checks that a direct read, a direct open, a default `ATTACH`, the `limits.attach`
constructor option, and the `db.limits.attach` setter are all unable to
reach the denied database, with the last two rejected by `ERR_ACCESS_DENIED`.
A companion `--allow-all` step is the positive regression guard: it
verifies that with full permissions both the option and the setter raise the limit and
`ATTACH` then succeeds, so a future change that over-tightens the cap
would fail CI.
Summary:
- defer freeing node:sqlite authorizer data while the SQLite callback is
still on the stack
- keep the existing authorizer registered when replacement validation
fails
- mark sqlite exec and prepare reentrant because authorizers can call
back into node:sqlite
Tests:
- `BINDGEN_EXTRA_CLANG_ARGS="-I/usr/lib/gcc/x86_64-linux-gnu/13/include"
cargo check -p deno_node_sqlite`
- `BINDGEN_EXTRA_CLANG_ARGS="-I/usr/lib/gcc/x86_64-linux-gnu/13/include"
cargo build -p deno -p test_server`
- `BINDGEN_EXTRA_CLANG_ARGS="-I/usr/lib/gcc/x86_64-linux-gnu/13/include"
cargo test -p unit_node_tests --test unit_node -- sqlite_test
--nocapture`
Closesdenoland/divybot#515
Co-authored-by: divybot <divybot@users.noreply.github.com>
Co-authored-by: Divy Srivastava <me@littledivy.com>
## Summary
`DatabaseSync.close()` finalized all prepared statements and dropped the
SQLite
connection without checking whether one of our user-defined callbacks
(`db.function`, `db.aggregate` step/inverse/value/final, authorizer,
`applyChangeset` filter/onConflict) was currently on the SQLite VDBE
stack.
Calling `db.close()` from inside an aggregate `step` callback freed the
in-flight statement and the connection while SQLite was still executing
— when
control returned to SQLite (and to our aggregate trampoline
`custom_aggregate_step_base`), it dereferenced the freed objects: a
native
**use-after-free**, reproducible with no permissions and no
`--unstable`.
This change:
- Adds a `callback_depth: Rc<Cell<usize>>` to `DatabaseSync` and an RAII
`CallbackDepthGuard` that bumps the counter at the top of every callback
trampoline (scalar function, aggregate step/value, authorizer) and
across
`sqlite3changeset_apply()`.
- Makes `DatabaseSync::close()` return a new `ERR_INVALID_STATE` SQLite
error
(`"cannot close database while a user-defined callback is running"`)
when
`callback_depth > 0`, instead of finalizing the in-flight statement.
- Marks `StatementSync#run`, `#all`, and `#iterate` as `#[reentrant]` so
the
new error thrown from inside a callback can propagate out through SQLite
rather than tripping the op-call reentrancy assertion.
The connection stays valid after a refused close.
## Reproducer
The original PoC now exits cleanly instead of crashing (SIGSEGV/SIGBUS
were
the typical pre-fix outcomes):
```js
import { DatabaseSync } from "node:sqlite";
const db = new DatabaseSync(":memory:");
db.exec("CREATE TABLE t(x); INSERT INTO t VALUES (1),(2),(3),(4),(5)");
let n = 0;
db.aggregate("a", {
start: 0,
step(acc, x) { if (++n === 2) db.close(); return acc; },
});
db.prepare("SELECT a(x) FROM t").get();
```
```
$ deno run poc.mjs
error: SqliteError: cannot close database while a user-defined callback is running
```
## Test plan
- [x] `cargo test -p unit_node_tests --test unit_node -- sqlite_test`
passes
with two new regression tests (aggregate step + scalar function).
- [x] PoC from the issue (`deno run poc.mjs`) throws `ERR_INVALID_STATE`
instead of crashing; verified across multiple runs (no flaky SIGSEGV).
- [x] `tools/lint.js` and `tools/format.js` clean.
Closesdenoland/orchid#330
Co-authored-by: divybot <divybot@users.noreply.github.com>
Co-authored-by: Divy Srivastava <me@littledivy.com>
## Summary
Upgrades rusty_v8 from 147.2.1 to 147.4.0. No Deno code changes needed.
### Performance improvements
- **simdutf string conversion hot paths** (`to_rust_string_lossy`,
`to_rust_cow_lossy`, `write_utf8_into`, `ValueView::to_cow_lossy`):
Latin-1 strings 2-4x faster, UTF-16 3-6x faster, ASCII unchanged.
Already wired up via the `simdutf` feature flag.
- **IsolateAnnex pointer caching** in scope structs reduces indirection
on every scope operation.
### Other changes
- feat: `Module::EvaluateForImportDefer` and `GetModuleNamespace` with
phase (for future `import.defer()` support)
- fix: enable linux shared-library-safe v8 TLS mode by default
- refactor: store `IsolateHandle` data in its own allocation
Fixes 3 SQLite Node.js compat tests from #32706:
- **`prepare()` options**: Support `readBigInts`, `returnArrays`,
`allowBareNamedParameters`, and `allowUnknownNamedParameters` as second
argument to `DatabaseSync.prepare(sql, options)`
- **Defensive mode default**: Changed from `false` to `true` to match
Node.js
- **Iterator invalidation**: Calling `get()`, `all()`, `run()`, or
`iterate()` on a statement now invalidates any active iterator, causing
`next()` to throw `ERR_INVALID_STATE` with "iterator was invalidated"
### Enabled tests
- `test-sqlite-config.js`
- `test-sqlite-named-parameters.js`
- `test-sqlite-statement-sync.js`
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
I let claude loose on improving compile times and it suggested splitting
up `deno_node` by extracting the crypto stuff and node sqlite. And the
other change is getting rid of all unused constants in `winerror.rs`.
That changes it from 17k lines down to 240.
| What | Time | Delta |
|---|--:|--:|
| main | 214.6s | - |
| PR | **162.8s** | **-51.8s** |
## Summary of changes
1. deno_node compile: -13.4s (48%) - Splitting out crypto and sqlite
plus removing 16K lines of unused winerror constants nearly halved
compile time.
2. deno_runtime: -5.2s (36%) - Less debug info to process.
3. Parallelism gains - deno_node_crypto (8.3s) and deno_node_sqlite
(2.5s) now compile concurrently with other work instead of being
serialized inside the monolithic deno_node.
Total CPU time for node crates
- Before: deno_node = 50.1s (both passes)
- After: deno_node (27.5s) + deno_node_crypto (16.5s) + deno_node_sqlite
(4.7s) = 48.6s
- Similar CPU time, but wall-clock is much better due to parallelism.
