Validate echo length; use virtual thread for ICE listener

[juerghegglin]

Two small unrelated cleanups around the ICE peer listener

1. PeerConnectivityCheckerModule.echoReceived() — early return on bad length

When an echo packet's length != 9, the code increments invalidEchosReceived and logs a trace, but then falls through to:

int rtt = (int) (System.currentTimeMillis()
        - Longs.fromByteArray(Arrays.copyOfRange(data, offset + 1, length)));

• If length < 9 → Longs.fromByteArray throws IllegalArgumentException (it requires exactly 8 bytes).
• If length > 9 → the first 8 bytes after the type prefix are reinterpreted as a timestamp, the resulting bogus RTT poisons the EMA at line 102, and subsequent averageRTT reporting becomes corrupted.

Adds an early return so invalid echoes only update the counter and don't compute an RTT.

2. PeerIceModule.startIce() — virtual thread for the data listener

PR #65 migrated this project to virtual threads (the connectivity checker, the telemetry sending loop, and the GPGNet/RPC acceptors are all Thread.ofVirtual()). The data listener inside PeerIceModule was missed — it still uses new Thread(this::listener), which means every peer holds a carrier thread blocked on socket.receive() for no benefit.

Replaces with Thread.ofVirtual().name("ice-listener-" + peer.getRemoteId()).start(this::listener) to match the surrounding code.

Risk

Both changes are local and self-contained. No behavior change other than the bug fix in (1). The listener thread continues to be assigned to the same volatile Thread listenerThread field and gets interrupted/joined exactly the same way on close.

Verification

Local Spotless + check pass on the fix branch. No JUnit tests added in this PR — happy to follow up with one for the echo-length validation if maintainers want it (would land alongside the JUnit infrastructure I proposed in the cover note of #84).

---

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

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Enhanced peer connectivity validation to properly detect and reject malformed echo responses, preventing corruption of round-trip time measurements and improving connection reliability.

• Performance

  • Optimized internal threading for peer connectivity management to reduce resource overhead and improve efficiency during connection establishment and monitoring.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3e9dabf7-1d31-4a02-9a9c-252c6fd88bb8

📥 Commits

Reviewing files that changed from the base of the PR and between d49f9fe and 837c709.

📒 Files selected for processing (2)

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

---

📝 Walkthrough

Walkthrough

The PR improves the ICE adapter with two independent enhancements: adding validation to reject invalid echo responses in connectivity checking, and upgrading the ICE listener thread from a platform thread to a virtual thread for better resource utilization.

Changes

ICE Adapter Improvements

Layer / File(s)	Summary
Input Validation ice-adapter/src/main/java/com/faforever/iceadapter/ice/PeerConnectivityCheckerModule.java	Early return added in echoReceived() to skip RTT calculation when received echo length is not 9, incrementing the invalid echo counter.
Threading Infrastructure ice-adapter/src/main/java/com/faforever/iceadapter/ice/PeerIceModule.java	ICE listener thread creation upgraded from platform Thread to virtual thread via Thread.ofVirtual() with explicit thread naming.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A rabbit hops through code so clean,
With echoes checked and threads so lean,
Invalid paths now skip on through,
Virtual threads bring speeds so new!
Two fixes hop toward the light, ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes both main changes: echo length validation and virtual thread migration for the ICE listener.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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.}

[juerghegglin]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.