fix(locking): EntityLocker atomicity + scan-lifecycle honesty (#231, #232) #241

Merged

timothy merged 4 commits from batch/231-232-lock-soundness into main 2026-07-11 13:05:16 +02:00

Conversation 0 Commits 4 Files Changed 21

+1252 -314

4 Commits

Author	SHA1	Message	Date
timothyandClaude Opus 4.8	702b0121cd	fix(review): bounded scan-pending net + scheduler compensating unlock (#232) ... Adversarial review (fork + Codex, both flagged) of PR #241: - SPA (both reviewers): removing PENDING_GRACE_TICKS wholesale reintroduced a stuck scan button. A 202'd scan that finishes between 10s polls (short/empty library) is never observed active, so its optimistic pending flag wedged the button disabled until reload. Restore a BOUNDED grace net (pruneGraceExpiredPending) — re-scoped honestly: it absorbs the inherent queue->observed-active lag and the fast-completion race, NOT the removed lying-200 compensation (the POST now returns 409/404/422 honestly). Bounds pending to PENDING_GRACE_TICKS * pollMs (~30s). - SPA 409 (Codex): on "already scanning" the button was cleared+reconciled, but a scan-status still lagging the in-progress scan re-enabled the button and let the user fire repeated 409s. Keep the pending flag on 409 (no toast) so the button stays disabled; polling promotes or expires it. - Scheduler (Codex): the Plex-Shows tail-token batch and the local/Jellyfin/Emby scan enqueues had no compensating unlock — a WriteAsync failure after LockLibrary (cancellation on shutdown) stranded the library lock. Wrap each acquired-lock enqueue in try/catch → UnlockLibrary → rethrow (the Plex catch covers both writes, since the library message carries Unlock: false and the un-enqueued networks message was the sole releaser). Tests: two new App.test.tsx cases — grace-window expiry re-enables the button, and 409 keeps it disabled through the queue->active lag. Ref #232. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-11 12:54:48 +02:00
timothy	78e4a3c738	Merge branch 'slice/232-scan' into batch/231-232-lock-soundness	2026-07-11 12:39:04 +02:00
timothyandClaude Opus 4.8	088644e8c0	feat(libraries): honest scan lifecycle + one-lock-one-release discipline (#232) ... Scan queue handler now returns a QueueLibraryScanResult enum (Queued|NotFound|SyncDisabled|AlreadyScanning) instead of a lying bool; LibrariesController.ScanLibrary maps them to 202/404/422/409 with ProblemDetails. Guard the lock->enqueue with the EnqueueWithTraktLock compensating-unlock pattern. ScannerService now releases every library/collection lock in a finally so a handler exception can't leak the lock. Plex "Shows" scheduler batch (one lock, two messages) now has only the trailing SynchronizePlexNetworks carry the single release (Unlock flag), mirroring the scheduler Trakt tail-token precedent. Guard the other lock->enqueue producers (Create/UpdateLocalLibrary, UpdateTraktList) with compensating unlock. SPA drops the PENDING_GRACE_TICKS heuristic now that the POST reports 202/409/404/422 directly: 202 -> pending+poll, 409 -> reconcile (no error toast), 404/422 -> surface error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-11 12:38:08 +02:00
timothyandClaude Opus 4.8	275908ec11	fix(locking): atomic EntityLocker flags + single-owner release contract (#231) ... The six plain-bool lock flags (Plex, Trakt, Emby/Jellyfin/Plex collections, troubleshooting playback) used a non-atomic check-then-set, so two concurrent Lock* callers could both win. Convert them to int flags mutated only via Interlocked.CompareExchange, so the caller that wins the 0->1 transition is the sole owner and the only one that fires the change event. The three ConcurrentDictionary-backed kinds (Library/Playout/RemoteMediaSource) were already atomic; drop their redundant ContainsKey pre-checks. Define the ownership contract (tokenless single-owner discipline, no interface change) on IEntityLocker and in docs/decisions.md: a true from Lock* confers ownership of exactly one release; Unlock* on an unlocked slot returns false, fires no event, and logs a warning (the double-release / non-owner tripwire). Adds EntityLockerTests (real locker, parallel-caller races) proving exactly one winner per kind, one-releaser-per-slot, and event-fires-once-per-transition. Ref #231. Scan-lifecycle call-site fixes that consume this contract land in the same PR (#232); the BuildPlayout/subtitle finally-gating is #234. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-11 12:30:36 +02:00