Use System.Threading.Lock on NET9_0_OR_GREATER

[mtmk]

Replace `private readonly object` lock fields with `System.Threading.Lock` under `#if NET9_0_OR_GREATER`. On .NET 9+, the C# compiler emits `Lock.EnterScope()` calls instead of `Monitor.Enter`/`Exit`, avoiding the object-header overhead. The `lock (x)` syntax is unchanged.

Nine lock fields updated across core (CommandWriter, NatsConnection, NatsSubBase, SubscriptionManager, ObjectPool, ReplyTask) and JetStream (NatsJSConsume, NatsJSOrderedConsume, NatsJSConsumer). Downlevel TFMs fall through to the existing `object` path.

[synadia-claude-reviewer]

Claude finished @mtmk's task in 6m 46s —— View job

---

PR Review: Use System.Threading.Lock on NET9_0_OR_GREATER

• Gather context (CLAUDE.md, PR diff, changed files)
• Review correctness of conditional compilation approach
• Check thread safety and reentrancy implications
• Review each changed file for issues
• Post final review summary

---

Summary

The overall approach is sound: the project's TFM list (netstandard2.0;netstandard2.1;net8.0;net10.0) means NET9_0_OR_GREATER is active for the net10.0 build, where System.Threading.Lock is available and the C# compiler (with LangVersion=latest) will correctly emit Lock.EnterScope() instead of Monitor.Enter/Exit. Field declarations and lock() usage are consistent across all 9 files.

However, there is one correctness concern worth investigating before merging.

---

Issue: SubscriptionManager._gate is relied upon as reentrant

System.Threading.Lock is not reentrant — on .NET 9+, EnterScope() throws LockRecursionException if the same thread tries to re-acquire the lock it already holds. Monitor.Enter (used by lock (object)) silently allows recursion.

There is a live re-entrancy path in the existing code:

SubscriptionManager.PublishToClientHandlersAsync (line 103) acquires SubscriptionManager._gate and then — still inside the lock — calls return sub.ReceiveAsync(...). Because ReceiveAsync is async ValueTask, its synchronous prefix runs immediately while the gate is held.

Inside that synchronous prefix, before the first await, NatsSubBase.ReceiveAsync can call EndSubscription:

// NatsSubBase.ReceiveAsync — synchronous portion, executed while SubscriptionManager._gate is held
case { StopOnEmptyMsg: true }:
    EndSubscription(NatsSubEndReason.EmptyMsg);   // ← synchronous
    return;

// Also triggered via SetException → EndSubscription for ThrowIfNoResponders + 503 responses
case { ThrowIfNoResponders: true } when IsHeader503(headersBuffer):
    SetException(new NatsNoRespondersException()); // ← synchronous
    return;

EndSubscription (line 484) calls UnsubscribeAsync() synchronously (fire-and-forget pattern, discarding the ValueTask). UnsubscribeAsync is not async; its body runs immediately and ends with:

return _manager.RemoveAsync(this);   // synchronous call into SubscriptionManager

SubscriptionManager.RemoveAsync (line 173) then tries to lock (_gate) — the same SubscriptionManager._gate already held on this thread → LockRecursionException on .NET 9+.

Under the old object lock this was invisible: Monitor.Enter simply incremented the recursion count.

Affected scenarios:

• RequestManyAsync — uses StopOnEmptyMsg = true (ReplyManyOptsDefault, NatsConnection.RequestReply.cs:18). Receives a zero-byte sentinel message after the result stream ends.
• Any subscription with ThrowIfNoResponders = true that receives a NATS 503 "No Responders" status message (e.g. RequestAsync when no subscribers exist, ReplyOptsDefault line 13).

Both are common, non-pathological code paths.

Fix suggestion →

---

Everything else looks correct

• All 9 #if NET9_0_OR_GREATER blocks are properly guarded with #else / #endif.
• No Monitor.TryEnter, Monitor.Enter(ref bool), or other low-level patterns are used on the replaced fields — only the lock() statement, which compiles to Lock.EnterScope() on .NET 9+.
• The lockHeld parameters in CommandWriter's state machine methods guard a SemaphoreSlim (_semLock), not any of the replaced lock fields — no conflict.
• ReplyTask._gate is initialized in the constructor (new System.Threading.Lock()), correctly mirroring the old new object().
• All other lock (_gate) usages across the changed files are sequential (not nested on the same call stack).
• The performance intent is correct: changing the static type of the field to System.Threading.Lock is what causes the compiler to emit Lock.EnterScope() rather than Monitor.Enter — leaving the field typed as object would not get the optimization even at runtime.

[github-actions]

🚀 Deployed on https://69f201ee904c556a0686964a--teal-klepon-7e92d0.netlify.app

[synadia-claude-reviewer]

Claude finished @mtmk's task in 2m 3s —— View job

---

PR Review: Use System.Threading.Lock on NET9_0_OR_GREATER

• Gather context and read CLAUDE.md
• Review the PR diff and changed files
• Check the reentrancy fix (commit 85c17a6) for correctness
• Post final review summary

---

Summary

The PR looks correct and ready to merge. The reentrancy issue flagged in the previous review has been fixed properly in commit 85c17a6.

---

Reentrancy Fix — Verified Correct

The previous review identified that SubscriptionManager.PublishToClientHandlersAsync called sub.ReceiveAsync(...) while holding _gate, which could chain into EndSubscription → UnsubscribeAsync → _manager.RemoveAsync → lock(_gate), causing LockRecursionException on .NET 9+.

The fix in SubscriptionManager.cs:103–134 is correct:

• targetSub is captured inside lock (_gate)
• The lock is released before targetSub.ReceiveAsync(...) is called
• This breaks the reentrancy chain entirely

The post-lock window (where another thread could dispose targetSub between capture and ReceiveAsync) is safe: we hold a strong reference via TryGetTarget, and ReceiveAsync already handles ChannelClosedException gracefully for this kind of race.

---

Everything Else Checked

• CommandWriter: lock (_lock) protects only _socketConnection/_ctsReader/_readerLoopTask in short, non-nested blocks. The lockHeld parameter throughout refers to _semLock (a SemaphoreSlim) — no conflict with the replaced _lock field.
• PartialSendFailureCounter._gate: Nested private class, used only for simple counter protection. No cross-lock nesting.
• ObjectPool, NatsConnection, NatsSubBase, NatsJSConsume, NatsJSOrderedConsume, NatsJSConsumer: All lock() usages on the replaced fields are sequential, non-reentrant. _pendingGate in the JetStream consumers is isolated and never nested with NatsSubBase._gate or SubscriptionManager._gate.
• ReplyTask._gate: Field declared without inline initializer, correctly initialized as new System.Threading.Lock() in the constructor.
• All 9 #if NET9_0_OR_GREATER / #else / #endif guards are syntactically correct.
• NET9_0_OR_GREATER is active for the net10.0 TFM in the project's target framework list, where System.Threading.Lock is available.

[mtmk]

Benchmarks run on this branch (BenchmarkDotNet, Release build, Windows):

Raw lock microbench (LockBench):

Method	Runtime	Mean	Ratio
ObjectLock_Empty	.NET 8.0	20.31 ns	1.00
SystemLock_Empty	.NET 8.0	20.32 ns	1.00
ObjectLock_Empty	.NET 10.0	20.42 ns	1.00
SystemLock_Empty	.NET 10.0	17.87 ns	0.88

On .NET 8 both rows use object (the #if guard is inactive), so they're identical -- expected. On .NET 10 System.Threading.Lock saves ~2.5 ns per acquire/release, about 12% on the raw operation.

Serial publish throughput (PublishRuntimeBench, 100k msgs, 128-byte payload):

Runtime	Mean
.NET 8.0	35.35 ms
.NET 10.0	32.32 ms

~9% faster on .NET 10 overall. The lock change contributes a fraction of that -- at ~3M msg/s the 2.5 ns saving per lock cycle adds up to well under 1% of total publish time. The bulk of the runtime delta is JIT and pipeline improvements.

[MauriceVanVeen]

LGTM