Skip to content

Guard disconnect callback reentry and maximum replay sequence - #13

Merged
gafferongames merged 8 commits into
sprint/netcode-cs-portfrom
sprint/netcode-cs-port-block.w5.g2.e21
Oct 15, 2026
Merged

gafferongames merged 8 commits into
sprint/netcode-cs-portfrom
sprint/netcode-cs-port-block.w5.g2.e21

Conversation

@gafferongames

Copy link
Copy Markdown
Contributor

Disconnect callbacks may safely disconnect their own slot or stop the server without recursive callbacks, negative counts, or an invalid encryption index. The same protection covers loopback clients. Replay protection now rejects the reserved maximum sequence, matching the C reference.

This integrates the landed token-lifetime/address-selection block and preserves its strict expiry floor. Red-first unit and C differential checks cover reentry, stop/restart, timeout, loopback, maximum sequence, and expiry boundaries. Linux build and full runner passed; all 12 C interoperability legs passed against 39f04965. Parent and independent source review cover the final head.

Head: 52f0dd4
Branch: sprint/netcode-cs-port-block.w5.g2.e21
Verdict: LAND-ready for parent review and publication; no push, PR, merge into stream, or sprint finish done here.
Base integrated: sprint/netcode-cs-port at 7e2663c, which contains block 2 head da890bf.
C reference: 39f049657d6e738e533c751c2b5242fa7b2d4114, mas-bandwidth/netcode sprint/netcode-audit-fixes.
Scope: 7 files changed relative to the landed stream, 447 insertions, 3 deletions; clean checkout and git diff --check.

Subtask 1 — disconnect callback reentry:

  • Red test: test_server_disconnect_callback_reentry_is_harmless on Linux failed with System.IndexOutOfRangeException in Server.SendClientPacket/EncryptionManager.Touch after a callback disconnected its own slot. Commit 411f023.
  • Red C differential with byte-identical request/response inputs: C printed callbacks 1 for same, stop, timeout and loopback; C# printed callbacks 2 for each and connected -1 for loopback. Commit f745b6a.
  • Fix: mark active disconnect slots, skip recursive disconnect, let Stop clear active slots without a second callback, return after callbacks that stopped/cleared the server, guard the encryption index. Loopback receives the same protection. Commit 9befda5.
  • Green: named test passed; full runner passed; interop leg 11 matched C, including one callback per case, expected reason/id/user data, zero connected/mappings, clear slot, and restart with an empty slot.

Subtask 1 gate lines:

RED test_server_disconnect_callback_reentry_is_harmless: Unhandled exception. System.IndexOutOfRangeException
  at Netcode.EncryptionManager.Touch(Int32 index, Address& address, Double time)
RED C:   same callbacks 1 saw 3 4369 90 connected 0 mappings 0 slot clear
RED C#:  same callbacks 2 saw 3 4369 90 connected 0 mappings 0 slot clear
RED C#:  loopback callbacks 2 saw 3 4369 90 connected -1 mappings 0 slot clear
GREEN test_server_disconnect_callback_reentry_is_harmless: All tests passed.
GREEN interop leg 11: disconnect callback reentry differential matched

Subtask 2 — replay sentinel and restart floor:

  • Red tests before integration: test_replay_protection failed at AlreadyReceived(ulong.MaxValue); test_connection_keep_alive_packet failed to refuse a valid max-sequence packet; test_connection_request_packet failed to refuse expiry equal to floor. Commit f685d7a.
  • Integrated the verified block 2 stream head through merge ade80f4. It already contains the C-parity expiry <= floor fix and its token-lifetime/fixture changes. Conflict resolution preserved both existing interop modes and this card's modes; the duplicate equal-floor unit assertion was replaced by block 2's wider request boundary test. After merge, the floor test passed while the two replay tests remained red. The disconnect unit fixture was adapted to block 2's max-lifetime/floor policy.
  • Red C differential with byte-identical keepalive and three request files: C printed before/after UINT64_MAX -> 1 and keepalive -> refused; C# printed 0 and accepted. F-1/F/F+1 printed refused/refused/accepted on both sides. Commit 5cb716c.
  • Fix: ReplayProtection.AlreadyReceived now reserves ulong.MaxValue before the empty-slot lookup. Commit 398e8c8. Hardened disconnect differential to fail on callback exceptions and a retained slot after restart in commit 52f0dd4.
  • Green: replay, keepalive and request named tests passed; full runner passed; interop leg 12 matched C across all sequence probes and three expiry boundaries.

Subtask 2 gate lines:

RED test_replay_protection: check failed: replayProtection.AlreadyReceived(ulong.MaxValue)
RED test_connection_keep_alive_packet: check failed: result.Type == -1
RED test_connection_request_packet: check failed: result.Type == -1
RED C:  before 18446744073709551615 -> 1; after 18446744073709551615 -> 1; keepalive 18446744073709551615 -> refused
RED C#: before 18446744073709551615 -> 0; after 18446744073709551615 -> 0; keepalive 18446744073709551615 -> accepted
GREEN test_replay_protection, test_connection_keep_alive_packet, test_connection_request_packet: All tests passed.
GREEN interop leg 12: replay/floor differential matched

Final Linux gate (nova@b1, /home/nova/stella-netcode-block1-e21; nice 10, max 2 build workers, job-local TMPDIR):

  • dotnet build tests/Tests.csproj -c Release -p:UseAppHost=false -maxcpucount:2: exit 0, 0 warnings, 0 errors. See logs/build-final.log.
  • dotnet run -c Release --project tests/Tests.csproj -f net8.0 --no-build -p:UseAppHost=false: exit 0, All tests passed. See logs/tests-final.log.
  • sh scripts/interop.sh /home/nova/stella-netcode-block1-e21/c-reference: exit 0, all 12 legs passed, INTEROP GATE PASSED. See logs/interop-final.log.
  • The seven changed source files have matching SHA-256 values between the job checkout and Linux bench. The C checkout's Git tip is the exact pinned reference above.

Remaining work: parent-owned independent cold reads, publication and sprint landing. No task code remains unfinished.

gafferongames and others added 8 commits October 15, 2026 02:23
…st for callback reentry

By: Stella

Co-Authored-By: GPT-6 Sol <noreply@openai.com>
…ential test against the C reference

By: Stella

Co-Authored-By: GPT-6 Sol <noreply@openai.com>
…active disconnect callbacks

By: Stella

Co-Authored-By: GPT-6 Sol <noreply@openai.com>
…replay sentinel and equal floor

By: Stella

Co-Authored-By: GPT-6 Sol <noreply@openai.com>
By: Stella

Co-Authored-By: GPT-6 Sol <noreply@openai.com>

# Conflicts:
#	compat/Compat.cs
#	compat/c/compat.c
#	scripts/interop.sh
#	tests/Tests.cs
…test against the C reference

By: Stella

Co-Authored-By: GPT-6 Sol <noreply@openai.com>
…y sentinel sequence

By: Stella

Co-Authored-By: GPT-6 Sol <noreply@openai.com>
By: Stella

Co-Authored-By: GPT-6 Sol <noreply@openai.com>
@gafferongames
gafferongames merged commit bc3c8ab into sprint/netcode-cs-port Oct 15, 2026
1 check passed
@gafferongames
gafferongames deleted the sprint/netcode-cs-port-block.w5.g2.e21 branch October 15, 2026 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant