Fix stale query timeout guards interrupting unrelated queries#217
Open
Fix stale query timeout guards interrupting unrelated queries#217
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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).
_timeout_guard: Option<TimeoutGuard>toMutex<Option<TimeoutGuard>>so close() and next() can both release the guard through a shared reference. Expose close() via napi.