Update SwappableLock to support NET9+ Lock type#1077
Update SwappableLock to support NET9+ Lock type#1077dwcullop wants to merge 1 commit 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>
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.
Description:
SwappableLock is a ref struct used internally by ObservableCache to atomically swap between lock objects during write operations. It currently only supports Monitor.Enter/Monitor.Exit on object gates.
.NET 9 introduced System.Threading.Lock: a dedicated, more efficient lock type that doesn't support Monitor.Enter. This PR adds overloads so SwappableLock works with both object (pre-.NET 9) and Lock (.NET 9+).
Changes
Uses #if NET9_0_OR_GREATER conditional compilation. Zero behavioral change on pre-.NET 9 targets.
Why this matters
ObservableCache needs to swap locks atomically during write operations (e.g., from the notification lock to the writer lock). Without Lock support, .NET 9+ code paths that use Lock elsewhere would be incompatible with SwappableLock, forcing fallback to object locks and losing the performance benefits of the new Lock type.