PT-4159: Rebuild S/R write gate without thread affinity

[lyonsil]

PT-4159 — proposed follow-up to #2555, targeting that PR's branch so the diff shows only the write-gate rewrite.

Summary

Rebuilds SendReceiveWriteLock from a ReaderWriterLockSlim wrapper into a single-atomic-word design: an armed flag, an in-flight write count, and a 30-bit arm generation packed into one long, with every transition (enter/exit write, arm, disarm) a single Interlocked read-modify-write on that word. The exclusion invariant — no project write can overlap a sync's file-replacement window — now follows from single-location total ordering: a write can only enter by atomically observing not armed while incrementing the count, and arming atomically sets the flag, so every write either drains before the sync proceeds or is rejected with the (SR_EDIT_BLOCKED) sentinel. SetSyncing returns the generation as an arm token (never 0, so default is a safe no-arm sentinel): Clear(token) ends only the bracket that owns the sync slot (a stale token is a logged no-op, so a late or duplicate Clear cannot disarm a newer sync), while parameterless Clear() remains the unconditional crash-recovery path.

Why review this

Part of epic PT-4158 (automatic Send/Receive). A concurrency deep dive on #2555's gate found that its correctness rests on a thread-affinity contract — SetSyncing/Clear on one thread, no await across the bracket or a write scope — that is documented but unenforceable, must be honored by the closed-source PT-4210 activation this repo cannot see or test, and whose every violation ends in a permanently held write lock (process-wide edit rejection until restart). See the discussion threads on #2555, especially the design-level question on SetSyncing, plus findings 4, 5, 6, 11, 20, the _writeLockHeld redundancy thread, and the NoRecursion trap thread — all structurally addressed here.

Changes

• SendReceiveWriteLock.cs: the rewrite. Unchanged semantics: fail-fast sentinel rejection, bounded 10 s drain, degraded proceed-on-timeout by default (new writes still rejected), global gate, per-project IsBlocked, inert in public core. The activation API keeps its SetSyncing/Clear shape (SetSyncing now returns the arm token; Clear(token) is the bracket-scoped variant). The contract:
  • SetSyncing/Clear may run on any threads; an await across the bracket is safe; Clear() is idempotent and doubles as crash recovery (a watchdog is now possible — RWLS thread affinity forbade one); Clear(token) makes a late bracket-end a logged no-op instead of a silent disarm of a newer sync
  • write scopes may cross an await and dispose on another thread; double-dispose and over-release are guarded no-ops
  • nested EnterWrite does not crash (no NoRecursion trap), but it is NOT rejection-safe — a sync arming while the outer scope is open rejects the inner call mid-mutation — so one scope per mutation (the SetBookUsfmInScope delegation pattern) is the required shape for delegating writes
  • the drain waits via SpinWait.SpinUntil and ends early (with a warning) if the arm is cleared or replaced mid-drain; the timeout warning reports the stuck in-flight count; optional SetSyncing(ids, throwOnDrainTimeout: true) rolls the arm back and throws TimeoutException instead of proceeding degraded, so a scheduler can retry/defer/notify
  • armed-ness is an explicit flag (closes the empty-batch degraded-path hole); the project set is built before any state is touched (no torn arm); null/empty ids are ignored, and a batch with no valid ids arms fail-safe with a logged warning; StringComparer.OrdinalIgnoreCase replaces hand-rolled case folding
  • no "is the lock held" bookkeeping bool exists to fall out of step with reality
• SendReceiveWriteLockTests.cs: new-contract tests, written first wherever a red was observable (nesting while unarmed and while armed, cross-thread Clear with the armed premise asserted, cross-thread scope dispose, null-id tolerance, token round-trip / stale-token no-op / idempotent re-Clear, prompt return when a Clear lands mid-drain, throw-on-timeout rollback, recovery while a write is stuck including the straggler's own decrement). Assertions target InFlightWriteCount/ArmedProjectIds so release, double-dispose, and id-filter regressions are directly falsifiable, and the fixtures reset the gate's full state (including the in-flight count) between tests. Two old-contract tests removed, FakeSync helper deleted (no thread affinity → tests arm directly). The concurrency stress test passes unmodified.
• CLAUDE.md: the "Send/Receive Write Gate" thread-affinity rules replaced with the new contract — token-ended brackets, the nesting hazard, and keep-scopes-tight guidance.
• ParatextProjectDataProvider.cs: comments updated to the new contract (single-scope delegation is required, not stylistic; the ParatextData write lock is named explicitly to avoid confusion with the gate's scope). Code unchanged.

AI Involvement

AI-assisted — session; an earlier session's URL was not captured. The concurrency analysis, the design, the implementation, and the tests were AI-generated with my direction and review; the TDD red/green evidence and full-suite runs were verified in-session. Commits carry Co-authored-by trailers.

Testing

• TDD: new-contract tests were observed failing for the expected reasons before the implementation made them pass — against the RWLS design (LockRecursionException on nesting, cross-thread Clear InvalidOperationException, SynchronizationLockException on cross-thread dispose, NRE on a null id) and against the unpatched drain (a Clear mid-drain burned the full DrainTimeout)
• Full dotnet test c-sharp suite: 1511 passed / 0 failed / 6 skipped (baseline 1500; delta is exactly the net new tests); includes the untouched SendReceiveWriteLockCoverageTests enforcement scan and all gate wiring tests
• csharpier clean; no TypeScript changes
• PT-4210 activation review: confirm the patch's bracket usage — end brackets with Clear(token); nothing enforces this, and parameterless Clear() force-disarms any live sync (it is the crash-recovery hammer). Decide whether the scheduler opts into throwOnDrainTimeout: true, and double-check the overlap semantics (one global sync slot; the scheduler must serialize sync runs)

Risk Level

Low–Medium — the gate is inert in public core (nothing calls SetSyncing), so public behavior is unchanged; the risk is confined to the PT-4210-activated path, where the forgiving contract widens what the activation may safely do and token-checked Clears make late bracket-ends harmless.

Merge note

Targets pt-4159-auto-sync-edit-block (#2555). If #2555 squash-merges first, retarget this PR to main and rebase. Squash-merge is fine for this PR (no template changes).

🤖 Generated with Claude Code

---

This change is