Skip to content

fix(sqlite): stop SqliteAiVectorStorage.putBulk deadlocking on its own chain slot - #846

Merged
sroussey merged 1 commit into
claude/multi-repo-transactions-708from
claude/branch-security-review-xs0tph-libs-vector-deadlock
Aug 20, 2026
Merged

fix(sqlite): stop SqliteAiVectorStorage.putBulk deadlocking on its own chain slot#846
sroussey merged 1 commit into
claude/multi-repo-transactions-708from
claude/branch-security-review-xs0tph-libs-vector-deadlock

Conversation

@sroussey

Copy link
Copy Markdown
Collaborator

Summary

SqliteAiVectorStorage.putBulk takes the shared connection-chain slot twice, and the inner take awaits a promise that only resolves after the inner call returns. Four vector tests hang for their full 60 s timeout, and the poisoned handle then hangs afterEach's deleteAll() on a 15 s hook timeout.

This removes the redundant inner take, restoring symmetry with SqliteTabularStorage._putBulkInternal.

Root cause

Two things independently take the chain slot for one logical write:

  • the public putBulk takes it in guardedWrite (added with the connection-mutex work), and
  • _putBulkInternal took it again via runOnConnection (present since 3bfca03fd, when it was the only take and therefore correct).

The inner runOnConnection awaits the outer holder's promise. That promise resolves only when the outer call returns — which cannot happen until the inner one does. A call arriving through the tx proxy is in the same position: it runs inside a transaction that already holds the slot.

Whoever reaches _putBulkInternal already holds the slot, so it must not re-take it.

-    const dbWithFlag = this.database as unknown as { readonly inTransaction?: boolean };
-    const alreadyInTx = this.inTransaction || dbWithFlag.inTransaction === true;
-    const handle = this.connectionHandle();
-    if (handle !== null && !alreadyInTx) {
-      return runOnConnection(handle, this, () => this.runVectorPutBulkOnHandle(entities));
-    }
     return this.runVectorPutBulkOnHandle(entities);

1 file changed, 4 insertions(+), 12 deletions(-).

Provenance

This is a pre-existing regression on this branch, not a defect in #845. It was introduced by 1eef8dbb8 — the first commit of this branch — and stayed hidden because the branch's build-types job was broken, so every test job was skipped.

Bisect over bun scripts/test.ts vitest integration storage:

commit result
2a9aeba80 (parent) 21 passed, 4.8 s
1eef8dbb8 4 failed, 218.7 s
ff4b97196 4 failed
4b22a8545 4 failed
4b22a8545 + this fix 21 passed

Verification

Red/green on this exact tree, same command, fix stashed and restored:

Without the fix

× putBulk + similaritySearch returns inserted vectors at distance 0        60059ms (retry x1)
× putBulk persists vectors as Float32Array via getAll                      60060ms (retry x1)
× mixed put + putBulk yields identical similaritySearch ordering           60078ms (retry x1)
× does not deadlock external putBulk queued behind a running withTransaction 34034ms (retry x1)
Error: Hook timed out in 15000ms.
Error: deadlock-timeout
Test Files  1 failed | 25 passed (26)
     Tests  4 failed | 1275 passed | 31 skipped (1310)

With the fix

Test Files  26 passed (26)
     Tests  1279 passed | 31 skipped (1310)

46.29 s.

Note the fourth failing test — does not deadlock external putBulk queued behind a running withTransaction — is the branch's own regression test for this property, failing on the branch that added it.


Generated by Claude Code

…n chain slot

`guardedWrite` (added with withConnectionTransaction) routes every public
write through `runOnConnection`, but `SqliteAiVectorStorage._putBulkInternal`
had taken the shared connection-chain slot itself since before anything above
it did. `putBulk` therefore took the slot twice on one handle: the inner
`runOnConnection` awaits the outer's promise, which only resolves once the
inner returns.

The `finally` never runs, so `state.chain` is left unresolved and the handle
is poisoned for every later write on it — which is why the suite's `afterEach`
`deleteAll()` timed out alongside the three `putBulk` tests.

The parent `SqliteTabularStorage._putBulkInternal` does not re-take the chain;
this restores that symmetry. Both entry paths already hold the slot: the public
`putBulk` via `guardedWrite`, and a call arriving through the `tx` proxy runs
inside the transaction that holds it — which is why the removed branch was
reachable only from the deadlocking path. `runVectorPutBulkOnHandle` still
reads `db.inTransaction` to decide whether to open its own `db.transaction`,
so SQLite transaction nesting is unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 59.81% 32927 / 55052
🔵 Statements 59.64% 34056 / 57098
🔵 Functions 61.7% 6360 / 10307
🔵 Branches 48.47% 16423 / 33876
File CoverageNo changed files found.
Generated in workflow #3244 for commit 1d08185 by the Vitest Coverage Report Action

@sroussey
sroussey merged commit 159af0b into claude/multi-repo-transactions-708 Aug 20, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant