fix(store): open the journal's connection under its lock; answer "Ready" only while paused #33

2026-11-10T22:54:09Z

james.maes commented

2026-11-10 22:54:09 +00:00

Closes audit finding W24 and tightens update preparation.

Store. Microsoft.Data.Sqlite 10.0.0 has a moment, between taking a pooled connection and recording its owner, in which the pool's own reclaim treats the connection as leaked. A second thread opening the same journal at that moment could be handed the same native handle, and the service opens its store from several tasks on one signal. The worst case traced was an autocommit write undone by another store's transaction, or a job reported FAILED or OUTCOME_UNKNOWN needlessly; no path to a reprint. The store now opens its connection under the journal's schema lock, so no opener of one journal can be in that gap while another asks the pool. The constructor and docs/DEVELOPMENT.md say why the open must stay there, with the library version and the methods to re-read before changing the package or pooling.

Update preparation. "Ready for update." is answered only while the pause is still in force and no sign-in, sign-out or enrollment is in progress; otherwise the reply says preparation was interrupted and to try again, which the updater already treats as not ready. This no longer depends on the pipe serving one command at a time.

Tests. A deterministic test fails if the open moves back outside the lock. A stress test (eight threads opening one journal together) reproduced the original fault before the fix at 4,000 rounds (3 of 3 runs) and is kept at 400 rounds, where it is a working check rather than a dependable detector; its summary says so. Two tests for interrupted preparation; the heartbeat test helper no longer depends on the clock sitting at an interval boundary.

Verification, local, on this head (on 1dfad0b): build/Verify.ps1 passed three times: 793 Core and 609 Windows/command-line tests, zero warnings; Core 100%, command line 100%, Windows 70.2 to 70.4%.

Review: independent; the reviewer checked the argument against the library source at v10.0.0 and found nothing at should-fix or above. Open nits, recorded for the test-harness step: raw test connections to a journal bypass the lock; the stress test costs more than it detects at its current length.

🤖 Generated with Claude Code

Closes audit finding W24 and tightens update preparation. **Store.** Microsoft.Data.Sqlite 10.0.0 has a moment, between taking a pooled connection and recording its owner, in which the pool's own reclaim treats the connection as leaked. A second thread opening the same journal at that moment could be handed the same native handle, and the service opens its store from several tasks on one signal. The worst case traced was an autocommit write undone by another store's transaction, or a job reported FAILED or OUTCOME_UNKNOWN needlessly; no path to a reprint. The store now opens its connection under the journal's schema lock, so no opener of one journal can be in that gap while another asks the pool. The constructor and `docs/DEVELOPMENT.md` say why the open must stay there, with the library version and the methods to re-read before changing the package or pooling. **Update preparation.** "Ready for update." is answered only while the pause is still in force and no sign-in, sign-out or enrollment is in progress; otherwise the reply says preparation was interrupted and to try again, which the updater already treats as not ready. This no longer depends on the pipe serving one command at a time. **Tests.** A deterministic test fails if the open moves back outside the lock. A stress test (eight threads opening one journal together) reproduced the original fault before the fix at 4,000 rounds (3 of 3 runs) and is kept at 400 rounds, where it is a working check rather than a dependable detector; its summary says so. Two tests for interrupted preparation; the heartbeat test helper no longer depends on the clock sitting at an interval boundary. **Verification**, local, on this head (on `1dfad0b`): `build/Verify.ps1` passed three times: 793 Core and 609 Windows/command-line tests, zero warnings; Core 100%, command line 100%, Windows 70.2 to 70.4%. **Review**: independent; the reviewer checked the argument against the library source at v10.0.0 and found nothing at should-fix or above. Open nits, recorded for the test-harness step: raw test connections to a journal bypass the lock; the stress test costs more than it detects at its current length. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 4 commits 2026-11-10 22:54:09 +00:00

fix(store): open the journal's connection under its schema lock ec808ca7c9

Microsoft.Data.Sqlite 10.0.0 takes a pooled connection under the pool's
lock and records its owner only afterwards. When a second thread opens
the same journal in that gap and finds the pool empty, the pool takes
the first connection for abandoned and hands it out again, so two
stores share one native handle. The service opens stores from several
tasks on one signal, so this could roll back an autocommit write or
make a statement fail and a job be reported FAILED or OUTCOME_UNKNOWN
without need.

The store now opens its connection inside the journal's schema lock.
Every open of a journal goes through that constructor and the lock is
at least as wide as the pool, so no opener can be in the gap while
another asks the pool. The pool's other reclaim, in Clear, is reached
only by clearing pools, which the service never does.

One test shows the property directly and fails every time when the
open is outside the lock. A stress loop of stores opened together
failed in three runs of three at 4,000 rounds before the fix and in
none of four at 1,000; it is kept at 400 rounds as a working check.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

fix(update): answer "Ready" only while the pause holds and the connection is not changing e795c906b8

Update preparation releases the connection gate before it waits, so
its "Ready for update." rested on the management pipe serving one
command at a time, and the 2.0 pipe will serve several. When the wait
ends it now checks that the pause is still in force and that no
sign-in, sign-out or enrollment is under way. Otherwise it answers
that preparation was interrupted and must be tried again, and logs
that at WARN.

The two operations are internal so tests can call them side by side,
as a pipe that serves several commands will. Both new tests fail when
preparation always answers ready.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

test(journal): end the files with a newline; halve the stress loop 53890e46c5

JournalFilesTests.cs and AgentStoreOpenRaceTests.cs lacked the final
newline .editorconfig asks for. The journal stress loop spins on the
SQLite provider's global lock, so it is cut from 5,000 opens to 2,500:
with the cleanup clearing every pool that still failed in five runs of
six, where 1,500 failed in two of six.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

test(fixture): let a heartbeat round be asked for after the clock moved part of an interval 11d801fc24

HeartbeatAsync looked for a timer armed at the present instant, so a
test that had advanced the manual clock by less than a heartbeat
interval made the next call wait until its limit. The helper now finds
the heartbeat loop's wait by the length it was armed for and advances
by what remains of it. A test moves the clock seven seconds and then
asks for a round; with the old lookup it times out.

Signed-off-by: James Maes <james@kof22.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

james.maes merged commit 2375195937 into release/2.0

2026-11-10 22:54:11 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QRun-IO/qqq-print-agent#33