FW-69b: extract RestartSessionStore (session store + restart-id allocator)#42
Merged
Merged
Conversation
…-id allocator) Step 5 of FW-64 (store half; FW-69a was the journal). Behaviour-preserving extraction of the session persistence (sessions_ map) + restart-id allocator (nextRestartId_) into a RestartSessionStore that BORROWS the coordinator's lock_ (same pattern as FW-68's DuplexOperationGate), because sessions_ is co-mutated with the clock-request maps + the operation gate inside four multi-domain single-lock critical sections (RequestClockConfig, ClearSession, TryConsumePendingClockRequest, CompleteClockRequest). The store exposes self-locking methods (LoadSession/StoreSession/GetSession/AllocateRestartId, reproducing the former members) + lock-held accessors (FindSessionLocked const/non-const, EraseSessionLocked) that the four multi-domain holds call while already holding lock_, so each keeps its single uninterrupted IOLockLock/IOLockUnlock span. Coordinator entry points become thin forwarders; IsRestartEpochCurrent/ClearSession/RequestClockConfig/TryConsume/ CompleteClockRequest keep their exact lock structure and call the *Locked accessors. No atomicity/ordering change. Verified: RestartSessionStoreTests 9/9, DiceDuplexRestartCoordinatorTests 20/20, full host suite 1203/1203 (1194 + 9). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
FW-69b: extract RestartSessionStore (session store + restart-id allocator)
Step 5 of FW-64, store half — completes FW-69 (FW-69a was the journal). Behaviour-preserving,
no atomicity/ordering change.
What
Lifts the per-GUID session persistence (
sessions_map) and the restart-id allocator(
nextRestartId_) out of the coordinator intoAudio/Protocols/Backends/RestartSessionStore.hpp(class
RestartSessionStore, namespaceASFW::Audio::Backends). The four former membersLoadSession/StoreSession/GetSession/AllocateRestartIdbecome thin forwarders.Lock-borrowing — same pattern as the merged FW-68 gate
sessions_is read/mutated inside four multi-domain single-lock critical sections that alsotouch the clock-request maps and the operation gate:
RequestClockConfig,ClearSession,TryConsumePendingClockRequest,CompleteClockRequest(plusIsRestartEpochCurrent's own hold).A store owning its own lock would split those atomic sections. So — exactly as
DuplexOperationGatedoes — the store borrows the coordinator's
lock_(viaIOLock** = &lock_, never owns one)and exposes two tiers:
LoadSession/StoreSession/GetSession/AllocateRestartId— reproduce theformer members for standalone callers;
FindSessionLocked(const + non-const, returnsDiceRestartSession*/nullptr) andEraseSessionLocked— called by the multi-domain holds while already holdinglock_, so eachkeeps one uninterrupted
IOLockLock/IOLockUnlockspan.IsRestartEpochCurrent/ClearSession/RequestClockConfig/TryConsume/CompleteClockRequestkeeptheir exact lock structure and swap the inline
sessions_.find/sessions_.erasefor the*Lockedaccessors. The
FindSessionLockedpointer is used only within its lock hold andsessions_is notmutated under it, so it's exactly equivalent to the former
sessionIt->seconditerator use.Atomicity — verified at the diff level
Host tests stub
IOLockto a no-op, so in-hold atomicity can't be functionally tested — but it'sverifiable by inspection, and the diff makes it mechanical:
IOLocklines added, 8 removed — exactly the 4 forwarder'd store methods' lock+unlock. Thefive in-hold conversions touch zero lock lines. Every critical section keeps its span; the
*Lockedaccessors contain no locking (no double-lock), and no self-locking method is ever calledwhile
lock_is held (no deadlock).RestartSessionStoreis a class with in-class (implicitly inline) members, so no ODR hazard.Tests
New
RestartSessionStoreTests— 9 tests: load-miss →default{.guid}; store/load/get round-trip;get-miss → nullopt;
guid==0guards;AllocateRestartIdmonotonic from 1; null-lock short-circuit +fresh-read-each-call;
FindSessionLockedmutate-in-place visible via the self-locking read; find-miss→ nullptr + erase; const overload. The coordinator's 20 integration tests exercise the in-hold paths.
(A future nicety, not in this behaviour-preserving slice: an instrumented
IOLocktest double couldmachine-check "exactly one hold per multi-domain section" across the whole cluster, rather than by
diff review. Happy to add it as a separate change if useful.)
Verification
RestartSessionStoreTests: 9/9DiceDuplexRestartCoordinatorTests: still 20/20 (unchanged)🤖 Generated with Claude Code