Update SwappableLock to support NET9+ Lock type#1077
Update SwappableLock to support NET9+ Lock type#1077dwcullop wants to merge 3 commits intoreactivemarbles:mainfrom
Conversation
Add Lock overloads for SwappableLock.SwapTo and constructor to support the new System.Threading.Lock type on .NET 9+. Uses #if NET9_0_OR_GREATER conditional compilation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dwcullop
changed the title
There was a problem hiding this comment.
Pull request overview
Adds .NET 9+ System.Threading.Lock support to SwappableLock so ObservableCache can atomically swap between synchronization primitives without forcing object/Monitor locks on newer runtimes.
Changes:
- Added
CreateAndEnter(Lock gate)factory for NET9+. - Added
SwapTo(Lock gate)overload and updatedSwapTo(object gate)to correctly release whichever gate type is currently held. - Updated
Dispose()to release a heldLockon NET9+.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| #if NET9_0_OR_GREATER | ||
| public static SwappableLock CreateAndEnter(Lock gate) | ||
| { |
There was a problem hiding this comment.
On NET9+, the Lock overloads can still receive null at runtime (despite non-nullable annotations). Currently that would throw a NullReferenceException from gate.Enter()/Exit(), unlike the object overloads which throw ArgumentNullException. Consider adding an explicit null guard (e.g., ArgumentNullException.ThrowIfNull(gate)) in CreateAndEnter(Lock) to provide a consistent and clearer failure mode.
| { | |
| { | |
| System.ArgumentNullException.ThrowIfNull(gate); |
| public void SwapTo(Lock gate) | ||
| { | ||
| if (_lockGate is null && _gate is null) | ||
| throw new InvalidOperationException("Lock is not initialized"); | ||
|
|
||
| gate.Enter(); | ||
|
|
There was a problem hiding this comment.
SwapTo(Lock gate) should guard against a null Lock argument as well; otherwise gate.Enter() will throw NullReferenceException. Adding an explicit ArgumentNullException for a null gate keeps behavior consistent with Monitor.Enter on the object overloads and produces a more actionable exception.
| #if NET9_0_OR_GREATER | ||
| public static SwappableLock CreateAndEnter(Lock gate) | ||
| { | ||
| gate.Enter(); | ||
| return new SwappableLock() { _lockGate = gate }; | ||
| } | ||
| #endif | ||
|
|
||
| public void SwapTo(object gate) | ||
| { | ||
| #if NET9_0_OR_GREATER | ||
| if (_gate is null && _lockGate is null) | ||
| throw new InvalidOperationException("Lock is not initialized"); | ||
| #else | ||
| if (_gate is null) | ||
| throw new InvalidOperationException("Lock is not initialized"); | ||
| #endif | ||
|
|
||
| var hasNewLock = false; | ||
| Monitor.Enter(gate, ref hasNewLock); | ||
|
|
||
| #if NET9_0_OR_GREATER | ||
| if (_lockGate is not null) | ||
| { | ||
| _lockGate.Exit(); | ||
| _lockGate = null; | ||
| } | ||
| else | ||
| #endif | ||
| if (_hasLock) | ||
| Monitor.Exit(_gate); | ||
| { | ||
| Monitor.Exit(_gate!); | ||
| } | ||
|
|
||
| _hasLock = hasNewLock; | ||
| _gate = gate; | ||
| } | ||
|
|
||
| #if NET9_0_OR_GREATER | ||
| public void SwapTo(Lock gate) | ||
| { | ||
| if (_lockGate is null && _gate is null) | ||
| throw new InvalidOperationException("Lock is not initialized"); | ||
|
|
||
| gate.Enter(); | ||
|
|
||
| if (_lockGate is not null) | ||
| _lockGate.Exit(); | ||
| else if (_hasLock) | ||
| Monitor.Exit(_gate!); | ||
|
|
||
| _lockGate = gate; | ||
| _hasLock = false; | ||
| _gate = null; | ||
| } |
There was a problem hiding this comment.
This PR adds new NET9+ behavior (CreateAndEnter(Lock), SwapTo(Lock), and mixed-type swapping/release logic) but there are no tests covering it. Since the test project targets net9.0, it should be feasible to add unit tests that validate swapping object→Lock, Lock→object, and Dispose releasing the correct gate.
… tests Replace #if spaghetti throughout every method with two complete, independent implementations behind a single top-level #if NET9_0_OR_GREATER. NET9: uses System.Threading.Lock directly (simpler, no _hasLock bool needed). Pre-NET9: uses Monitor.Enter/Exit on object gates (unchanged behavior). Filter.Dynamic: gates upgraded to Lock on NET9 for consistency. Added SwappableLockFixture with 7 tests covering CreateAndEnter, Dispose (release + idempotent), SwapTo (basic + chained), uninitialized throws, and Dispose after swap.
Problem
SwappableLockis aref structused internally byFilter.Dynamicto atomically swap between lock objects during filtered changeset processing. It originally only supportedMonitor.Enter/Monitor.Exitonobjectgates..NET 9 introduced
System.Threading.Lock: a dedicated, more efficient lock type that is non-reentrant and does not supportMonitor.Enter. Code paths that useLockon .NET 9 were incompatible withSwappableLock.Approach
Instead of sprinkling
#if NET9_0_OR_GREATERthroughout every method (mixingLockandobjectstate in the same struct), we splitSwappableLockinto two complete, independent implementations behind a single top-level#if:System.Threading.Lockdirectly. Simpler (no_hasLockbool needed,Locktracks its own state).Monitor.Enter/Exitonobjectgates. Unchanged behavior from the original.Each implementation is self-contained and readable without mental
#ifgymnastics.Changes
Internal/SwappableLock.cs- Two complete implementations behind top-level#if. NET9 version usesLock.Cache/Internal/Filter.Dynamic.cs- Gate properties returnLockon NET9,objectotherwise. DedicatedLockfields on NET9.Tests/Internal/SwappableLockFixture.cs- New. 7 tests covering CreateAndEnter, Dispose (release + idempotent), SwapTo (basic + chained), uninitialized throws, and Dispose after swap.Zero behavioral change on pre-.NET 9 targets.