Skip to content

Commit 241a359

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 241a359

5 files changed

Lines changed: 136 additions & 14 deletions

File tree

compat.js

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,12 +309,39 @@ class Statement {
309309
try {
310310
const { params, queryOptions } = splitBindParameters(bindParameters);
311311
const it = statementIterateSync(this.stmt, params, queryOptions);
312+
let closed = false;
313+
const close = () => {
314+
if (closed) {
315+
return;
316+
}
317+
closed = true;
318+
if (typeof it.close === "function") {
319+
it.close();
320+
}
321+
};
322+
const next = () => {
323+
try {
324+
const record = iteratorNextSync(it);
325+
if (record.done) {
326+
close();
327+
}
328+
return record;
329+
} catch (err) {
330+
close();
331+
throw err;
332+
}
333+
};
312334
return {
313-
next: () => iteratorNextSync(it),
314-
[Symbol.iterator]() {
335+
next,
336+
return(value) {
337+
close();
315338
return {
316-
next: () => iteratorNextSync(it),
317-
}
339+
done: true,
340+
value,
341+
};
342+
},
343+
[Symbol.iterator]() {
344+
return this;
318345
},
319346
};
320347
} catch (err) {

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+
const TIMEOUT = 500;
428+
const [db] = await connect(":memory:", { defaultQueryTimeout: TIMEOUT });
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 < 10000; 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 < 100; i++) {
443+
const rows = await stmt.all();
444+
t.is(rows.length, 10000);
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 & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -354,16 +354,41 @@ class Statement {
354354
try {
355355
const { params, queryOptions } = splitBindParameters(bindParameters);
356356
const it = await this.stmt.iterate(params, queryOptions);
357+
let closed = false;
358+
const close = () => {
359+
if (closed) {
360+
return;
361+
}
362+
closed = true;
363+
if (typeof it.close === "function") {
364+
it.close();
365+
}
366+
};
367+
const next = async () => {
368+
try {
369+
const record = await it.next();
370+
if (record.done) {
371+
close();
372+
}
373+
return record;
374+
} catch (err) {
375+
close();
376+
throw err;
377+
}
378+
};
357379
return {
358380
next() {
359-
return it.next();
381+
return next();
382+
},
383+
return(value) {
384+
close();
385+
return Promise.resolve({
386+
done: true,
387+
value,
388+
});
360389
},
361390
[Symbol.asyncIterator]() {
362-
return {
363-
next() {
364-
return it.next();
365-
}
366-
};
391+
return this;
367392
}
368393
};
369394
} catch (err) {

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,33 @@ impl RowsIterator {
14101410
safe_ints,
14111411
raw,
14121412
pluck,
1413-
_timeout_guard: timeout_guard,
1413+
timeout_guard: Mutex::new(timeout_guard),
14141414
}
14151415
}
14161416

1417+
fn release_timeout_guard(&self) {
1418+
let mut guard = self.timeout_guard.lock().unwrap();
1419+
guard.take();
1420+
}
1421+
1422+
#[napi]
1423+
pub fn close(&self) {
1424+
self.release_timeout_guard();
1425+
}
1426+
14171427
#[napi]
14181428
pub async fn next(&self) -> Result<Record> {
14191429
let mut rows = self.rows.lock().await;
1420-
let row = rows.next().await.map_err(Error::from)?;
1430+
let row = match rows.next().await {
1431+
Ok(row) => row,
1432+
Err(err) => {
1433+
self.release_timeout_guard();
1434+
return Err(Error::from(err).into());
1435+
}
1436+
};
1437+
if row.is_none() {
1438+
self.release_timeout_guard();
1439+
}
14211440
Ok(Record {
14221441
row,
14231442
column_names: self.column_names.clone(),

0 commit comments

Comments
 (0)