From 3b9ffac2fe90bb7d71cac2706258e5a701a3a2d1 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Wed, 30 Sep 2026 17:23:30 +0200 Subject: [PATCH] C#: a send failure's goodbye belongs to the generation that sent Send ended an unsendable session after releasing SessionGate, and the goodbye was built under the gate but sent outside it. If the polling thread admitted a new Connect from the same endpoint in that window, the ended generation's goodbye reached the new client: it saw Data, Accept, Disconnect and believed itself disconnected while the listener held it connected. Every path that finds a permanent send failure now reads the session's generation under the gate at the send, and EndUnsendableSession builds and sends the goodbye under the gate only while that generation is unchanged. The generation check replaces the UnsendableReason once-guard: a second failure in the same generation finds it already ended. The test orders the new Connect inside the send, before the fragment that can never be sent, and asserts the new client hears no Disconnect and stays connected on both sides. It fails on the pre-fix code (Data, Accept, Disconnect) and kills the mutant that drops the check. Co-Authored-By: Claude Opus 5.5 --- src/GameCult.Networking/CultNetTransport.cs | 40 ++++++++++++------ .../CultNetRudpSendFailureTests.cs | 41 +++++++++++++++++++ 2 files changed, 68 insertions(+), 13 deletions(-) diff --git a/src/GameCult.Networking/CultNetTransport.cs b/src/GameCult.Networking/CultNetTransport.cs index 7b09fe6a..24839d41 100644 --- a/src/GameCult.Networking/CultNetTransport.cs +++ b/src/GameCult.Networking/CultNetTransport.cs @@ -2841,10 +2841,12 @@ namespace GameCult.Networking if (peer == null) throw new ArgumentNullException(nameof(peer)); SocketException? unsendable = null; var failedIndex = 0; + long generation; lock (peer.SessionGate) { var firstSequence = peer.Session.NextSequence; var packets = peer.Session.SendMany(channelId, payload, ChannelSendOptions(channelId), _maxFragmentBytes); + generation = peer.Session.Generation; unsendable = SendPackets(peer, packets, out failedIndex); // A caller-directed send that can never succeed is the caller's error and queues // nothing. Only when earlier fragments are already on the wire has the peer seen @@ -2856,7 +2858,7 @@ namespace GameCult.Networking { // The polling thread retires the peer; this thread only ends its session. if (failedIndex > 0) - EndUnsendableSession(peer, unsendable); + EndUnsendableSession(peer, unsendable, generation); throw unsendable; } _stats.FramesSent++; @@ -2973,8 +2975,10 @@ namespace GameCult.Networking { var repeats = false; SocketException? unsendableReply = null; + long admittedGeneration; lock (admitted.SessionGate) { + admittedGeneration = admitted.Session.Generation; if (!admitted.Session.ConnectRepeats(packet)) { admitted.Session.ResetPeerState(); @@ -2998,7 +3002,7 @@ namespace GameCult.Networking if (repeats) { if (unsendableReply != null) - EndUnsendablePeer(admitted, unsendableReply); + EndUnsendablePeer(admitted, unsendableReply, admittedGeneration); return true; } } @@ -3044,6 +3048,7 @@ namespace GameCult.Networking CultNetRudpReceiveResult? outcome; CultNetRudpPacket? acknowledgement = null; SocketException? unsendable = null; + long existingGeneration = 0; lock (existingPeer.SessionGate) { try @@ -3065,6 +3070,7 @@ namespace GameCult.Networking } else { + existingGeneration = existingPeer.Session.Generation; if (outcome.Reply != null) unsendable = SendPacket(existingPeer.RemoteEndPoint, outcome.Reply); unsendable ??= SendPackets(existingPeer, outcome.ReadyToSend, out _); @@ -3107,7 +3113,7 @@ namespace GameCult.Networking if (acknowledgement != null && unsendable == null) unsendable = SendPacket(existingPeer.RemoteEndPoint, acknowledgement); if (unsendable != null) - EndUnsendablePeer(existingPeer, unsendable); + EndUnsendablePeer(existingPeer, unsendable, existingGeneration); return true; } @@ -3123,10 +3129,15 @@ namespace GameCult.Networking foreach (var peer in _peers.Values.ToArray()) { SocketException? unsendable; + long generation; lock (peer.SessionGate) - unsendable = SendPackets(peer, peer.Session.DueResends(NowMs()), out _); + { + var due = peer.Session.DueResends(NowMs()); + generation = peer.Session.Generation; + unsendable = SendPackets(peer, due, out _); + } if (unsendable != null) - EndUnsendablePeer(peer, unsendable); + EndUnsendablePeer(peer, unsendable, generation); } } @@ -3152,25 +3163,28 @@ namespace GameCult.Networking // An admitted peer owes a packet that can never be sent as built. Inside a poll that ends the // peer's session, never the poll: the peer is told, and PeerDisconnected carries a reason that // names the error. - private void EndUnsendablePeer(CultNetRudpSocketServerPeer peer, SocketException error) + private void EndUnsendablePeer(CultNetRudpSocketServerPeer peer, SocketException error, long generation) { - EndUnsendableSession(peer, error); + EndUnsendableSession(peer, error, generation); RetireUnsendablePeers(); } // Ends the session and tells the peer, on whichever thread found the failure. The peer stays in - // _peers until the polling thread retires it. - private void EndUnsendableSession(CultNetRudpSocketServerPeer peer, SocketException error) + // _peers until the polling thread retires it. The failure belongs to the generation the thread + // read under the gate when it sent; if that generation has already ended (another thread + // ended it over its own failure, or a new Connect from the endpoint replaced it), there is + // nothing left to end, and a goodbye would reach the session that now owns the endpoint. The + // goodbye is built and sent under the gate, so no Connect is admitted between the two. + private void EndUnsendableSession(CultNetRudpSocketServerPeer peer, SocketException error, long generation) { - CultNetRudpPacket goodbye; lock (peer.SessionGate) { - if (peer.UnsendableReason != null) + if (peer.Session.Generation != generation) return; - goodbye = peer.Session.EndUnsendable(error); + var goodbye = peer.Session.EndUnsendable(error); peer.UnsendableReason = goodbye.Payload; + SendPacket(peer.RemoteEndPoint, goodbye); } - SendPacket(peer.RemoteEndPoint, goodbye); _unsendablePeers.Enqueue(peer); } diff --git a/tests/GameCult.Networking.Tests/CultNetRudpSendFailureTests.cs b/tests/GameCult.Networking.Tests/CultNetRudpSendFailureTests.cs index 0ae010dc..c9287c12 100644 --- a/tests/GameCult.Networking.Tests/CultNetRudpSendFailureTests.cs +++ b/tests/GameCult.Networking.Tests/CultNetRudpSendFailureTests.cs @@ -313,6 +313,47 @@ namespace GameCult.Networking.Tests } } + // A send that fails after its first fragment ends the generation it began in, and only that + // one. Here X connects again while the send is under way, and the polling thread admits the + // new Connect before the fragment that can never be sent. The hook runs that poll inside the + // send so the order is fixed; in production it runs on the polling thread between the send + // and the failure's handling. The goodbye belonged to the ended generation, and the client + // must not hear it: it would believe itself disconnected while the listener holds it + // connected. + [Test] + public void ASendFailureAfterANewConnectReplacedThePeerSaysNoGoodbyeToTheNewSession() + { + var (server, listenerEndPoint, x, _) = TwoPeers(maxFragmentBytes: 1000); + using (server) + { + var stale = x.Server; + Drain(x.Socket); + x.Session = new CultNetRudpSession(new CultNetRudpSessionOptions { ConnectionId = ConnectionId }); + x.Socket.SendTo(CultNetRudpPacketCodec.Encode(x.Session.CreateConnect(0)), listenerEndPoint); + + var fragments = 0; + server.BeforeSend = (remote, packet) => + { + if (!remote.Equals(x.EndPoint) || packet.PacketType != CultNetRudpPacketType.Data || ++fragments < 2) + return; + server.BeforeSend = null; + server.ReceiveOnce(); + server.UnsendableAfter[x.EndPoint] = 0; + }; + Assert.Throws(() => server.SendSchema(stale, new byte[3000])); + server.ReceiveOnce(); + + var packets = Drain(x.Socket); + Assert.That(packets.Select(p => p.PacketType), Has.None.EqualTo(CultNetRudpPacketType.Disconnect), + "the ended generation's goodbye reached the session that replaced it"); + x.Session.Receive(packets.Single(p => p.PacketType == CultNetRudpPacketType.Accept), 0); + Assert.That(x.Session.Connected, Is.True); + var replacing = server.Peers.Single(p => p.RemoteEndPoint.Equals(x.EndPoint)); + Assert.That(replacing, Is.Not.SameAs(stale)); + Assert.That(replacing.Connected, Is.True); + } + } + [Test] public void ASessionEndedByACallerAndThePollingThreadTogetherSaysGoodbyeOnce() {