alkstore/tasks/sqlite-fix-watcher-death-subscribe.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
sqlite-fix-watcher-death-subscribe	Dead-watcher subscribe hang + initial-stat self-kill (review 004 items 2+3)	completed		moderate	medium	component	implementation	review-004-fixes sqlite-engine watcher

Description

Two bundled watcher-state defects in the SQLite engine, per docs/reviews/004-pre-release-general-review.md §"Findings recorded, not fixed" items 2 and 3 (the review says bundle them — both are watcher-state changes).

(a) Post-watcher-death subscribe hang (item 2). After the watcher thread exits (db-file replacement), subscribe() still enrolls new senders with no poller alive (substrate/watcher.rs:391-396): a listen() or stream subscribe() on the dead store parks forever, contradicting engine-sqlite.md's "on watcher death every subscriber closes" posture — which is enforced only for pre-death subscribers. Fix shape: a dead-store/dead-watcher sign that the new subscription paths consult — a state-machine change across store.rs/notify.rs/ stream.rs. New subscriptions on a dead store must fail closed (a typed error, the established failure vocabulary) rather than park.

(b) Initial-stat failure self-kill (item 3). An identity-stat failure at watcher start seeds (0, 0) (watcher.rs:164-170); the first successful later stat mismatches it and drives the death path, spuriously closing every subscriber on recovery from a transient stat hiccup. Rare, but the fix — an identity-unknown state instead of a false identity — is the same watcher-state family: the watcher treats identity as unknown until the first successful stat, and that first successful stat must not trigger the death path.

Test shapes: (a) kill the watcher (the db-file replacement seam or a cfg(test) death injection), then listen()/subscribe() — assert the typed failure, not a park; the pre-death subscribers' close-at- death pins stay green. (b) inject an initial stat failure, let the first successful stat land, assert the watcher survives and subscribers stay open.

Acceptance Criteria

• Subscribing (listen or stream) after watcher death fails closed with a typed error instead of parking — pinned by test
• Pre-death subscribers still close at watcher death — the existing pins green unchanged
• An initial identity-stat failure no longer seeds a false identity: the first successful stat does not trigger the death path and subscribers stay open — pinned by injected-failure test
• engine-sqlite.md's watcher-death posture text covers the new-subscriber arm (and the identity-unknown state if it changes observable behavior)
• cargo test -p alkstore-sqlite, clippy -D warnings, fmt clean; workspace gates green server-less

References

• docs/reviews/004-pre-release-general-review.md §"Findings recorded, not fixed" items 2–3 (the findings, the fix shapes, the bundling)
• docs/architecture/engine-sqlite.md (the watcher-death posture)
• alkstore-sqlite/src/substrate/watcher.rs (both sites), store.rs / notify.rs / stream.rs (the subscription paths)

Notes

Decisions of record the description didn't pin:

• The dead sign lives on SharedUpdateWatcher as an Arc<AtomicBool>, set by run_poll_loop itself just before the watcher-fatal return — it is set before the death-guard's senders clear (which happens after the loop returns), so the guard-side ordering is fixed by program order.
• SharedUpdateWatcher::subscribe became fallible (Result<_, String>) and checks two closed arms atomically with the enrollment under the senders lock: the dead sign and the taken watcher slot (the post-close enroll-onto-empty-watcher race of the same shape). Failing closed on the taken slot is new belt-and-braces beyond the finding's shape; the store's closed flag already guarded the normal path. Rationale for checking under the lock: a check-then-enroll done outside the lock could interleave between the death and the guard's clear and enroll a ghost.
• The substrate stays contract-blind: the failure is a String naming the death, mapped into the taxonomy engine-side — Error::Database (the opaque fallback with the source chain), the same shape the engine's closed-store checks use. Error::Closed was rejected: the contract docs scope that variant to receiver closed arms (try_recv/recv_timeout), not entry points.
• The initial-identity fix is an in-place re-derivation of the same code (Option<(u64, u64)> baseline vs. the (0, 0) seed); the death-log message keeps its precise diagnostic (now printing the captured baseline).
• The injected-failure test for (b) is the substrate-level drive (run_poll_loop with the db file absent at start — the initial stat genuinely fails; on non-unix stat_identity never fails, so the scenario cannot arise and the test is unix-gated like the other file-replacement drives); no cfg(test) injection seam was added to stat_identity itself. Aliveness after the first successful stat is proven by wake delivery relative to a pre-insert snapshot (failing identity stats fire on_change, so absolute wake counts are not a clean liveness signal).
• Also updated: engine-sqlite.md's watcher-failure posture bullet (both arms), notify.rs/stream.rs/store.rs module docs, open_tests' three direct watcher.subscribe() call sites (now .unwrap()d), and one PROVENANCE.md row (D-39 — the register wants divergence to the ported watcher recorded even though the task text didn't name it).

Summary

Both defects landed with tests:

• (a) Post-watcher-death subscribe hang: the poll loop sets the dead sign before exiting through the identity-replacement death path; SharedUpdateWatcher::subscribe fails closed on it (and on a taken watcher slot) atomically with enrollment under the senders lock; notify.rs::listen and stream.rs::spawn_bridge map the failure to Error::Database with the death named in the source chain. Pinned by four tests: substrate-level (subscribe_fails_closed_after_watcher_death, unix file-replacement drive) and engine-level (listen_after_watcher_death_fails_closed, subscribe_after_watcher_death_fails_closed — same drive through the store surface, error-shape + no-enrollment assertions). The pre-death subscriber close-at-death pins (watcher_death_closes_the_receiver_terminally, watcher_death_closes_the_subscription_terminally, and the substrate death-signal tests) are green unchanged — the death-signal substrate test also now pins the dead sign being set.
• (b) Initial-stat self-kill: the loop tracks the identity baseline as Option<(u64, u64)>; a failed startup stat leaves it unknown, the first successful stat captures it without driving death, and a later genuine mismatch still kills. Pinned by initial_stat_failure_does_not_seed_a_false_identity (run_poll_loop drive: absent file → unknown baseline → file created → no death + wake delivery lands → replacement → death fires; unix-gated).
• Verified: cargo build, cargo test (workspace, all green), cargo test -p alkstore-sqlite (201 tests), fuzz gate (cargo test --manifest-path fuzz/shared/Cargo.toml), clippy -D warnings clean, cargo fmt --check clean, rustdoc -D warnings doc gate clean.