C#: a send failure on an already-ended session says no second goodbye

3b9ffac2 claimed that the generation check subsumed the UnsendableReason
once-guard. It did not. A thread that reads the generation only after
the session has ended carries the ended session's own generation, and the
check lets it through. For example, the polling thread answers a Ping on a
session that a caller's send has just ended but that has not yet been
retired. When that Pong fails permanently, the thread says a second
goodbye and overwrites the first failure's reason (Soul pass 4, PR4).
EndUnsendableSession now also returns when the session has already ended.

Two tests:
- The polling thread answers a Ping on a session already ended, and the
  Pong can never be sent. Exactly one goodbye goes out, and the first
  failure's reason stands. The hook admits the Ping after the session
  ended and before the peer is queued for retirement. The test kills the
  mutant that drops the Ended check.
- A failed send's goodbye is sent under the gate. A polling thread
  admitting a new Connect from the endpoint waits on the gate, so the old
  generation's goodbye never follows the new session's Accept. The test
  kills the mutant that sends the goodbye outside the gate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Meta Vens 2026-09-30 20:19:19 +02:00
commit b37f898465
2 changed files with 81 additions and 3 deletions

View file

@ -3172,14 +3172,16 @@ namespace GameCult.Networking
// Ends the session and tells the peer, on whichever thread found the failure. The peer stays in
// _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
// ended it over its own failure, or a new Connect from the endpoint replaced it), or the
// thread read it only after the session had ended (a Pong or ack answered on an ended
// session the polling thread has not yet retired), there is nothing left to end, and a
// goodbye would reach the session that now owns the endpoint or repeat the first one. 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)
{
lock (peer.SessionGate)
{
if (peer.Session.Generation != generation)
if (peer.Session.Generation != generation || peer.Session.Ended)
return;
var goodbye = peer.Session.EndUnsendable(error);
peer.UnsendableReason = goodbye.Payload;

View file

@ -354,6 +354,82 @@ namespace GameCult.Networking.Tests
}
}
// A caller's send ends X's session over a fragment it could never send. Before the listener
// retires X, the polling thread answers X's Ping on the ended session, and that Pong can never
// be sent either. The polling thread read the generation only after the session ended, so the
// generation alone cannot tell it the session is gone. It says no second goodbye, and the
// first failure's reason stands. The hook admits the Ping while the first goodbye is on its way,
// after the session ended and before the peer is queued for retirement.
[Test]
public void AFailureAnsweringAPingOnAnAlreadyEndedSessionSaysNoSecondGoodbye()
{
var (server, listenerEndPoint, x, _) = TwoPeers(maxFragmentBytes: 1000);
using (server)
{
var ended = RecordDisconnects(server);
byte[]? firstReason = null;
server.BeforeSend = (remote, packet) =>
{
if (!remote.Equals(x.EndPoint) || packet.PacketType != CultNetRudpPacketType.Disconnect)
return;
server.BeforeSend = null;
firstReason = x.Server.UnsendableReason;
server.UnsendableAfter[x.EndPoint] = 0;
server.ReceiveOnce();
};
x.Socket.SendTo(CultNetRudpPacketCodec.Encode(x.Session.CreatePing(Encoding.UTF8.GetBytes("ping"))), listenerEndPoint);
server.UnsendableAfter[x.EndPoint] = 1;
Assert.Throws<SocketException>(() => server.SendSchema(x.Server, new byte[3000]));
server.ReceiveOnce();
var goodbyes = Drain(x.Socket).Where(p => p.PacketType == CultNetRudpPacketType.Disconnect).ToList();
Assert.That(goodbyes, Has.Count.EqualTo(1));
Assert.That(firstReason, Is.Not.Null);
Assert.That(x.Server.UnsendableReason, Is.SameAs(firstReason), "the second failure overwrote the first's reason");
Assert.That(ended, Is.EqualTo(new[] { x.Server }));
Assert.That(x.Server.DisconnectReason, Is.SameAs(firstReason));
}
}
// The goodbye for a failed send is sent under the session's gate. The polling thread, admitting
// a new Connect from X, waits on that gate, so the ended generation's goodbye can never follow
// the new session's Accept. The hook starts the polling thread just before the goodbye leaves
// and gives it a second to get ahead if nothing holds it back.
[Test]
public void AFailedSendsGoodbyeLeavesBeforeANewConnectFromTheEndpointIsAdmitted()
{
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);
Thread? poller = null;
server.BeforeSend = (remote, packet) =>
{
if (!remote.Equals(x.EndPoint) || packet.PacketType != CultNetRudpPacketType.Disconnect)
return;
server.BeforeSend = null;
poller = new Thread(() => server.ReceiveOnce());
poller.Start();
poller.Join(1000);
};
server.UnsendableAfter[x.EndPoint] = 1;
Assert.Throws<SocketException>(() => server.SendSchema(stale, new byte[3000]));
Assert.That(poller, Is.Not.Null);
poller!.Join();
var types = Drain(x.Socket).Select(p => p.PacketType).ToList();
Assert.That(types, Does.Contain(CultNetRudpPacketType.Disconnect));
Assert.That(types, Does.Contain(CultNetRudpPacketType.Accept));
Assert.That(types.IndexOf(CultNetRudpPacketType.Disconnect), Is.LessThan(types.IndexOf(CultNetRudpPacketType.Accept)),
"the ended generation's goodbye followed the new session's Accept");
}
}
[Test]
public void ASessionEndedByACallerAndThePollingThreadTogetherSaysGoodbyeOnce()
{