fix(store): set busy_timeout before journal_mode=WAL to prevent immediate SQLITE_BUSY#117
Merged
DeusData merged 1 commit intoDeusData:mainfrom Mar 23, 2026
Conversation
…iate SQLITE_BUSY on lock contention Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
QA Round 1 —
|
| # | Area | Severity | Result |
|---|---|---|---|
| 1 | Correctness: busy_timeout applies connection-wide before WAL pragma |
— | ✓ CORRECT — PRAGMA busy_timeout calls sqlite3_busy_timeout() on the connection; applies to all subsequent operations including DDL |
| 2 | Completeness: other sqlite3_open call sites |
INFO | ✓ All clear — cli.c config DB is single-owner (no WAL, no contention); dump path temp file is exclusively owned at pragma time |
| 3 | Scope: commit touches only src/store/store.c |
— | ✓ CLEAN — git show 804d680 --stat confirms 1 file, 6 lines |
| 4 | Test coverage: no busy_timeout assertion test |
MINOR | Gap exists — no test queries PRAGMA busy_timeout after cbm_store_open_path(). Not a blocker; could be a follow-up |
| 5 | Error handling: early return on busy_timeout failure |
— | ✓ CORRECT — WAL pragma never reached if busy_timeout fails |
| 6 | In-memory path skips busy_timeout and WAL |
— | ✓ CORRECT — :memory: DBs have no WAL mode and no concurrent access |
| 7 | SQLite documentation alignment | — | ✓ CONFIRMED — matches documented sqlite3_busy_timeout() semantics |
| 8 | Dump path WAL pragma (line 554, dest_db) |
— | ✓ NOT VULNERABLE — temp file is exclusively owned; no contention possible |
| 9 | Branch hygiene | — | ✓ CLEAN — single commit, correct format, no merge commits |
| 10 | Pre-existing: silenced error in dump path WAL pragma | INFO | Pre-existing pattern, out of scope for this PR |
Notes:
- The fix is mechanically correct.
PRAGMA busy_timeoutbeforePRAGMA journal_mode = WALensures the WAL lock acquisition can retry for up to 10 seconds rather than failing immediately. - The existing WAL invariant tests (
bulk_pragma_wal_invariant, etc.) still pass. The ordering change does not affect test behavior. - The
busy_timeouttest gap (Finding 4) could be addressed with a simple post-cbm_store_open_path()PRAGMA busy_timeoutquery assertion intest_store_*.c— suggested as a follow-up, not a merge requirement.
Owner
|
Merged to main. Fixes #116. Thanks @halindrome — correct and minimal fix. Setting busy_timeout before the WAL transition eliminates the zero-timeout window that caused SQLITE_BUSY under lock contention. |
6 tasks
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.
Summary
configure_pragmas()sobusy_timeout=10000is set beforejournal_mode=WALsqlite3_open_v2and the first pragma executionFixes #116
Root cause
sqlite3_open_v2returns a live connection with SQLite's default zero busy timeout. The prior ordering setbusy_timeoutas the third pragma — afterjournal_mode=WAL. Any lock contention during the WAL transition caused an immediateSQLITE_BUSYfailure with no retry.Change
Test plan
scripts/test.shpasses (all ~2040 cases)scripts/lint.shpasses (clang-tidy, cppcheck, clang-format)SQLITE_BUSYerror