Cover static-mode withholding of the connection packet (#64)

[Segfaultd]

Closes the remaining item on #64.

The gap was real

Before writing anything I checked whether #66 had already closed it, since the withholding mechanism is shared (SendSessionConfigResponse → ProduceWithheldConnectionPacket is the single release point, reached from both the static path and the interactive AcceptSession path). It had not.

A mutant that releases the server's withheld ID_NEW_INCOMING_CONNECTION at transport-up in static mode only, leaving the interactive path correctly gated:

memcpy(remoteSystem->withheldConnectionPacketData, data, byteSize);
remoteSystem->withheldConnectionPacketLength = byteSize;
}
/* MUTANT G: static-only early release -- interactive still gates */
if (sessionConfigInteractive==false)
    ProduceWithheldConnectionPacket(remoteSystem, (MessageID)ID_NEW_INCOMING_CONNECTION);

Passed all 23 tests in the suite. Static-mode gating was entirely unguarded.

Why it cannot be tested through received packets

In static mode the server answers the client's request the instant it arrives, so correct code and early-release code are indistinguishable to any poll — by the time the test looks, the payload it should have waited for has arrived anyway. This is exactly why the existing static tests, which assert the payload is readable once the connection packet surfaces, are satisfied by the mutant.

I also ruled out the alternatives rather than assuming:

• Plugin hooks. OnNewConnection fires from CallPluginCallbacks inside Receive() — the user thread, at dequeue time, not production time. It cannot witness the ordering. No hook observes AddPacketToProducer.
• The existing fake-socket harness. ReliabilityLayerBlackHoleTests drives two ReliabilityLayer objects directly; it never constructs a RakPeer, so it cannot reach the session handshake, which lives in RakPeer::RunUpdateCycle.
• A RakPeer-level fake socket. RakNetSocket2Allocator::AllocRNS2 is a static factory with no injection seam, and hand-crafting reliability-layer framing to feed OnRNS2Recv would be a large, fragile piece of infrastructure.

What the test does instead

Asserts the slot invariant — the connection is never reported to the application while the remote's payload is still missing — sampled continuously off the server's own RemoteSystemStruct, via a RakPeer-derived inspector. That is the same technique SendGateBypass in this file already uses to reach protected internals, with the same caveat documented (the peer must still come from GetInstance()).

The client's payload is 48 KB, so it splits across many datagrams and the handshake spends a long, reliably observable time in the gated state.

Two guards against the test quietly degrading:

• It asserts it actually witnessed the gated window (sawGatedState), so it cannot silently become a test that proves nothing — the failure mode that made the original gating test useless.
• It re-checks the 48 KB payload survived split and reassembly byte-for-byte.

Verification (macOS arm64)

Check	Result
Against the static-only mutant	fails 20/20, ~240 ms, ~4.8M sampled violations
Clean library, 30 repeats	30/30
Clean library, 4 parallel instances × 8	32/32
Full ctest	284/284

The detection signal is millions of samples wide, not a marginal timing catch.

Note

The test spins without sleeping, deliberately — the gated window is short and every sample counts, and it breaks out as soon as the connection is reported with the payload in hand (~240 ms typically). It does not poll Receive() during the loop, which is fine: the network thread does the work and Receive() only dequeues.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 26 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: b0a1173f-0d8c-4604-84ea-8c22e981b288

📥 Commits

Reviewing files that changed from the base of the PR and between 1c8438e and c226e35.

📒 Files selected for processing (1)

• Tests/Integration/SessionConfigLiveTests.cpp

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}