Skip to content

Commit 5596b09

Browse files
committed
Fix stale query timeout guards interrupting unrelated queries
The TimeoutGuard held by RowsIterator was only released when the JS garbage collector collected the native object. In a tight loop of stmt.all() calls (which uses iterate() internally), exhausted iterators would pile up unreachable but not yet GC'd. When their timeout deadline fired — potentially hundreds of milliseconds after the query finished — conn.interrupt() would kill whatever unrelated query happened to be running at that moment, producing spurious SQLITE_INTERRUPT errors well below the configured timeout. Fix: eagerly release the TimeoutGuard as soon as the iterator is exhausted (next() returns None), encounters an error, or is explicitly closed via the iterator return() protocol (break/throw in for...of). - Rust: change `_timeout_guard: Option<TimeoutGuard>` to `Mutex<Option<TimeoutGuard>>` so close() and next() can both release the guard through a shared reference. Expose close() via napi. - promise.js / compat.js: wrap the native iterator with an idempotent close helper and implement return() for early-termination cleanup. - Add integration test that reproduces the original bug: 100 sequential stmt.all() calls with a 500ms timeout, each finishing in ~115ms, which reliably triggered stale-guard interrupts before this fix.
1 parent 21485e8 commit 5596b09

File tree

5 files changed

+136
-32
lines changed

5 files changed

+136
-32
lines changed

compat.js

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -309,14 +309,7 @@ class Statement {
309309
try {
310310
const { params, queryOptions } = splitBindParameters(bindParameters);
311311
const it = statementIterateSync(this.stmt, params, queryOptions);
312-
return {
313-
next: () => iteratorNextSync(it),
314-
[Symbol.iterator]() {
315-
return {
316-
next: () => iteratorNextSync(it),
317-
}
318-
},
319-
};
312+
return wrappedIter(it);
320313
} catch (err) {
321314
throw convertError(err);
322315
}
@@ -331,11 +324,17 @@ class Statement {
331324
try {
332325
const result = [];
333326
const iterator = this.iterate(...bindParameters);
334-
let next;
335-
while (!(next = iterator.next()).done) {
336-
result.push(next.value);
327+
try {
328+
let next;
329+
while (!(next = iterator.next()).done) {
330+
result.push(next.value);
331+
}
332+
return result;
333+
} finally {
334+
if (typeof iterator.return === "function") {
335+
iterator.return();
336+
}
337337
}
338-
return result;
339338
} catch (err) {
340339
throw convertError(err);
341340
}
@@ -365,6 +364,26 @@ class Statement {
365364
}
366365
}
367366

367+
function wrappedIter(it) {
368+
return {
369+
next() {
370+
return iteratorNextSync(it);
371+
},
372+
return(value) {
373+
if (typeof it.close === "function") {
374+
it.close();
375+
}
376+
return {
377+
done: true,
378+
value,
379+
};
380+
},
381+
[Symbol.iterator]() {
382+
return this;
383+
},
384+
};
385+
}
386+
368387
module.exports = Database;
369388
module.exports.SqliteError = SqliteError;
370389
module.exports.Authorization = Authorization;

index.d.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,32 @@ export declare class Database {
8080
* - Legacy format: `{ [tableName: string]: 0 | 1 }`
8181
* - Full format: `{ rules: AuthRule[], defaultPolicy?: 0 | 1 | 2 }`
8282
* - `null` to remove the authorizer
83+
*
84+
* Pattern fields (`table`, `column`, `entity`) accept a plain string for
85+
* exact matching, or `{ glob: "pattern" }` for glob matching with `*` and `?`.
86+
*
87+
* # Examples
88+
*
89+
* ```javascript
90+
* const { Authorization, Action } = require('libsql');
91+
*
92+
* // Legacy table-level allow/deny
93+
* db.authorizer({ "users": Authorization.ALLOW });
94+
*
95+
* // Rule-based with glob patterns
96+
* db.authorizer({
97+
* rules: [
98+
* { action: Action.READ, table: "users", column: "password", policy: Authorization.IGNORE },
99+
* { action: Action.INSERT, table: { glob: "logs_*" }, policy: Authorization.ALLOW },
100+
* { action: Action.READ, policy: Authorization.ALLOW },
101+
* { action: Action.SELECT, policy: Authorization.ALLOW },
102+
* ],
103+
* defaultPolicy: Authorization.DENY,
104+
* });
105+
*
106+
* // Remove authorizer
107+
* db.authorizer(null);
108+
* ```
83109
*/
84110
authorizer(config: unknown): void
85111
/**
@@ -173,6 +199,7 @@ export declare class Statement {
173199
}
174200
/** A raw iterator over rows. The JavaScript layer wraps this in a iterable. */
175201
export declare class RowsIterator {
202+
close(): void
176203
next(): Promise<Record>
177204
}
178205
export declare class Record {

integration-tests/tests/async.test.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,30 @@ test.serial("Query timeout option allows short-running query", async (t) => {
423423
db.close();
424424
});
425425

426+
test.serial("Stale timeout guard from exhausted iterator does not interrupt later queries", async (t) => {
427+
t.timeout(30_000);
428+
const [db] = await connect(":memory:", { defaultQueryTimeout: 1_000 });
429+
430+
// Insert test data.
431+
await db.exec("CREATE TABLE t(x INTEGER)");
432+
const insert = await db.prepare("INSERT INTO t VALUES (?)");
433+
for (let i = 0; i < 2_000; i++) {
434+
await insert.run(i);
435+
}
436+
437+
// Run many sequential queries via stmt.all() (which uses iterate() internally).
438+
// Each query finishes well under the timeout, but if the RowsIterator's
439+
// TimeoutGuard is not released until GC, stale guards will fire and
440+
// interrupt unrelated later queries.
441+
const stmt = await db.prepare("SELECT * FROM t ORDER BY x ASC");
442+
for (let i = 0; i < 150; i++) {
443+
const rows = await stmt.all();
444+
t.is(rows.length, 2_000);
445+
}
446+
447+
db.close();
448+
});
449+
426450
test.serial("Per-query timeout option interrupts long-running Statement.all()", async (t) => {
427451
const [db, errorType] = await connect(":memory:");
428452
const stmt = await db.prepare(

promise.js

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -354,18 +354,7 @@ class Statement {
354354
try {
355355
const { params, queryOptions } = splitBindParameters(bindParameters);
356356
const it = await this.stmt.iterate(params, queryOptions);
357-
return {
358-
next() {
359-
return it.next();
360-
},
361-
[Symbol.asyncIterator]() {
362-
return {
363-
next() {
364-
return it.next();
365-
}
366-
};
367-
}
368-
};
357+
return wrappedIter(it);
369358
} catch (err) {
370359
throw convertError(err);
371360
}
@@ -380,11 +369,17 @@ class Statement {
380369
try {
381370
const result = [];
382371
const iterator = await this.iterate(...bindParameters);
383-
let next;
384-
while (!(next = await iterator.next()).done) {
385-
result.push(next.value);
372+
try {
373+
let next;
374+
while (!(next = await iterator.next()).done) {
375+
result.push(next.value);
376+
}
377+
return result;
378+
} finally {
379+
if (typeof iterator.return === "function") {
380+
await iterator.return();
381+
}
386382
}
387-
return result;
388383
} catch (err) {
389384
throw convertError(err);
390385
}
@@ -414,6 +409,26 @@ class Statement {
414409
}
415410
}
416411

412+
function wrappedIter(it) {
413+
return {
414+
next() {
415+
return it.next();
416+
},
417+
return(value) {
418+
if (typeof it.close === "function") {
419+
it.close();
420+
}
421+
return {
422+
done: true,
423+
value,
424+
};
425+
},
426+
[Symbol.asyncIterator]() {
427+
return this;
428+
}
429+
};
430+
}
431+
417432
module.exports = {
418433
Action,
419434
Authorization,

src/lib.rs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ use std::{
3434
str::FromStr,
3535
sync::{
3636
atomic::{AtomicBool, Ordering},
37-
Arc,
37+
Arc, Mutex,
3838
},
3939
time::Duration,
4040
};
@@ -1391,7 +1391,7 @@ pub struct RowsIterator {
13911391
safe_ints: bool,
13921392
raw: bool,
13931393
pluck: bool,
1394-
_timeout_guard: Option<TimeoutGuard>,
1394+
timeout_guard: Mutex<Option<TimeoutGuard>>,
13951395
}
13961396

13971397
#[napi]
@@ -1410,14 +1410,23 @@ impl RowsIterator {
14101410
safe_ints,
14111411
raw,
14121412
pluck,
1413-
_timeout_guard: timeout_guard,
1413+
timeout_guard: Mutex::new(timeout_guard),
14141414
}
14151415
}
14161416

14171417
#[napi]
14181418
pub async fn next(&self) -> Result<Record> {
14191419
let mut rows = self.rows.lock().await;
1420-
let row = rows.next().await.map_err(Error::from)?;
1420+
let row = match rows.next().await {
1421+
Ok(row) => row,
1422+
Err(err) => {
1423+
self.release_timeout_guard();
1424+
return Err(Error::from(err).into());
1425+
}
1426+
};
1427+
if row.is_none() {
1428+
self.release_timeout_guard();
1429+
}
14211430
Ok(Record {
14221431
row,
14231432
column_names: self.column_names.clone(),
@@ -1426,6 +1435,16 @@ impl RowsIterator {
14261435
pluck: self.pluck,
14271436
})
14281437
}
1438+
1439+
#[napi]
1440+
pub fn close(&self) {
1441+
self.release_timeout_guard();
1442+
}
1443+
1444+
fn release_timeout_guard(&self) {
1445+
let mut guard = self.timeout_guard.lock().unwrap();
1446+
guard.take();
1447+
}
14291448
}
14301449

14311450
/// Retrieve next row from an iterator synchronously. Needed for better-sqlite3 API compatibility.

0 commit comments

Comments
 (0)