Update iceState to DISCONNECTED in PeerIceModule.close() (fixes #59)

[juerghegglin]

Problem (closes #59)

Issue #59 reports a state desync where three things are simultaneously true:

• the game shows no connection to the peer
• the ICE debug window shows the peer's state as Connected
• the logs report SocketException: Socket closed

The original report includes stack traces from Peer.java and PeerIceModule.java listener loops that fire after the underlying socket has been closed — but the adapter's iceState field is still CONNECTED.

Root cause

PeerIceModule.close() does the cleanup work (interrupts the listener thread, closes the TURN refresh module, frees the ice4j Agent, stops the connectivity checker) but does not:

• set connected = false
• call rpcService.onConnected(..., false)
• call setState(DISCONNECTED) (which would notify both RPC and the telemetry/debug pipeline)
• null out the agent, mediaStream, and component references after agent.free()

So whatever iceState was when close() ran — typically CONNECTED because close() is invoked either from the RPC disconnect-from-peer path on a live peer or from GameSession.close() during game teardown — stays put. The FAF client never gets a state-change notification matching the actual closed-socket reality, the telemetry server's UI shows the peer as still connected, and any code path reading peer.getIce().getIceState() returns CONNECTED for a peer whose socket is gone.

The companion method onConnectionLost() does this correctly already. close() is the outlier.

Change

Rewrite PeerIceModule.close() to mirror onConnectionLost()'s cleanup-then-notify-then-free flow:

     void close() {
         if (listenerThread != null) {
             listenerThread.interrupt();
             listenerThread = null;
         }
         if (turnRefreshModule != null) {
             turnRefreshModule.close();
+            turnRefreshModule = null;
         }
+        connectivityChecker.stop();
+
+        // Mirror the state notifications that onConnectionLost() sends so the FAF client
+        // and telemetry see this peer as fully disconnected. Without this, iceState stayed
+        // at whatever it was (typically CONNECTED) when the peer was closed via the RPC
+        // disconnect path or game-end teardown - the desync reported in issue #59
+        // ("Socket closed, when state Connected").
+        if (connected) {
+            connected = false;
+            rpcService.onConnected(IceAdapter.getId(), peer.getRemoteId(), false);
+        }
+        if (iceState != DISCONNECTED) {
+            setState(DISCONNECTED);
+        }
+
         if (agent != null) {
             agent.free();
+            agent = null;
+            mediaStream = null;
+            component = null;
         }
-        connectivityChecker.stop();
     }

The connected and iceState checks prevent firing duplicate notifications when close() runs after onConnectionLost() has already updated state (e.g. peer drop followed by game-end cleanup).

Risk

Low. The new behavior is exactly what onConnectionLost() already does for the same fields. The state machine is unchanged for paths that go through onConnectionLost(); only the close() path now reports its actual outcome.

Side benefit

The dangling-reference cleanup (nulling agent, mediaStream, component after free(), plus turnRefreshModule after its close()) matches what onConnectionLost() does and reduces the chance the listener thread sees half-freed state during shutdown — partially defusing the known race that the catch (NullPointerException) in PeerIceModule.listener() exists to defend against.

Verification

• Local :ice-adapter:check (compile + spotlessCheck) passes after spotlessApply.
• The change pattern (mirroring onConnectionLost's state notification) is identical to existing well-tested code paths.

Closes #59.

---

Co-authored with Claude (Anthropic); reviewed and locally verified by me.

[coderabbitai]

Warning

Rate limit exceeded

@juerghegglin has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 51 minutes and 50 seconds before requesting another review.

To continue reviewing without waiting, purchase usage credits in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dbccbca5-65c2-46e6-b001-b2958e269ff8

📥 Commits

Reviewing files that changed from the base of the PR and between d49f9fe and 62146fa.

📒 Files selected for processing (1)

• ice-adapter/src/main/java/com/faforever/iceadapter/ice/PeerIceModule.java

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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 and usage tips.}