fix(events): graceful unregister no longer masquerades as overflow under FailFast

src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionEventDistributor.cs

@@ -683,14 +683,35 @@ public sealed class SessionEventDistributor : IAsyncDisposable
     }
 
     // Applies the per-subscriber backpressure policy when a subscriber's bounded channel is
-    // full. Runs on the pump thread. The offending subscriber is ALWAYS disconnected with an
-    // overflow fault and unregistered, so it can never wedge the pump again; the overflow
-    // handler decides the observable side effects (overflow metric, and — for legacy
+    // full — or, indistinguishably from the pump's side, already completed. A subscriber that
+    // really overflowed is ALWAYS disconnected with an overflow fault and unregistered, so it
+    // can never wedge the pump again; one that merely unregistered itself is dropped silently
+    // (see the discriminator below). Runs on the pump thread. The overflow handler decides the
+    // observable side effects (overflow metric, and — for legacy
     // single-subscriber FailFast — faulting the owning session). Multi-subscriber FailFast
     // intentionally degrades to a plain disconnect (see SubscriberOverflowHandler docs): one
     // slow consumer must not fault a session shared by other healthy subscribers.
     private void OnSubscriberOverflow(Subscriber subscriber, ulong workerSequence)
     {
+        // Claim the disconnect FIRST, because a false TryWrite is ambiguous. It means either
+        // "channel full" (a genuine overflow) or "channel already completed" — which happens
+        // when the subscriber unregistered after the pump captured the fan-out array and is
+        // therefore a GRACEFUL close, not backpressure. RemoveSubscriber separates the two:
+        // every path that completes a channel during fan-out (lease disposal via Unregister,
+        // and this method) removes the subscriber from the set BEFORE completing it, so a
+        // completed channel implies the subscriber is already gone and RemoveSubscriber
+        // returns false. (CompleteAllSubscribers completes without removing, but only after
+        // the pump has left its loop, so it cannot be observed here.)
+        //
+        // Bailing out on false is what keeps a normal stream ending mid-traffic from emitting
+        // a bogus EventQueueOverflow metric and — under the default single-subscriber FailFast
+        // policy — faulting the whole session. Winning the removal also guarantees the side
+        // effects below run exactly once per subscriber.
+        if (!RemoveSubscriber(subscriber))
+        {
+            return;
+        }
+
         // Decide whether FailFast may fault the whole session for this overflow. This is the
         // "isOnlySubscriber" signal the legacy single-subscriber FailFast path keys on.
         bool isOnlySubscriber = !subscriber.IsInternal && _singleSubscriberMode;
@@ -717,20 +738,15 @@ public sealed class SessionEventDistributor : IAsyncDisposable
                 subscriber.Id);
         }
 
-        // Disconnect ONLY this subscriber: complete its channel with the overflow fault and
-        // remove it from the fan-out set. Its gRPC reader's MoveNextAsync then throws the
-        // SessionManagerException, which EventStreamService surfaces to the client exactly as
-        // the pre-epic per-RPC overflow did. The pump and every other subscriber are untouched.
-        //
-        // The removal takes _lifecycleLock because it must republish the fan-out snapshot; the
-        // pump holds no other lock here (fan-out runs outside _replayLock), so this cannot
-        // invert the _replayLock-then-_lifecycleLock order RegisterWithReplay uses.
-        if (RemoveSubscriber(subscriber))
-        {
-            subscriber.Channel.Writer.TryComplete(new SessionManagerException(
-                SessionManagerErrorCode.EventQueueOverflow,
-                $"Session {_sessionId} event stream queue overflowed."));
-        }
+        // Disconnect ONLY this subscriber: it is already out of the fan-out set (removed above),
+        // so complete its channel with the overflow fault. Its gRPC reader's MoveNextAsync then
+        // throws the SessionManagerException, which EventStreamService surfaces to the client
+        // exactly as the pre-epic per-RPC overflow did. The pump and every other subscriber are
+        // untouched. This runs even when the handler above threw — the subscriber must never be
+        // left attached with an un-completed channel.
+        subscriber.Channel.Writer.TryComplete(new SessionManagerException(
+            SessionManagerErrorCode.EventQueueOverflow,
+            $"Session {_sessionId} event stream queue overflowed."));
     }
 
     private void CompleteAllSubscribers(Exception? error)
@@ -765,6 +781,11 @@ public sealed class SessionEventDistributor : IAsyncDisposable
     // Returns true only for the caller that actually removed it, so the channel is completed
     // exactly once however many disposal/overflow paths race. Completing the channel is left to
     // that caller and happens OUTSIDE the lock: this lock guards set membership only.
+    //
+    // Remove-then-complete (never the reverse) is load-bearing, not incidental: it is what lets
+    // OnSubscriberOverflow read a false return as "this subscriber unregistered gracefully"
+    // rather than "this subscriber overflowed". Completing before removing would resurrect the
+    // spurious-session-fault bug.
     private bool RemoveSubscriber(Subscriber subscriber)
     {
         lock (_lifecycleLock)