Skip to content

Commit 27212fd

Browse files
jheadcursoragent
andcommitted
Reply OfflinePong immediately so offline remotes stay on LAN.
Synchronous recovery probes delayed the offline advertisement past console discovery timeouts on blackholed hosts; probe health in the background instead. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent b4dc8e4 commit 27212fd

5 files changed

Lines changed: 174 additions & 34 deletions

File tree

‎internal/proto/proto.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ var OfflinePong = UnconnectedPing{
5959
ProtocolVersion: "390",
6060
Version: "1.14.60",
6161
Players: "0",
62-
MaxPlayers: "0",
62+
// Non-zero capacity: some consoles omit full/zero-slot entries from Friends.
63+
MaxPlayers: "1",
6364
GameType: "Creative",
6465
NintendoLimited: "1",
6566
// Placeholder ports so rewriteUnconnectedPong overwrites them with

‎internal/proxy/discovery_ping_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,3 +130,62 @@ func TestBuildOfflinePongEchoesPingTime(t *testing.T) {
130130
require.Equal(t, proto.UnconnectedPongID, pong[0])
131131
require.True(t, bytes.Equal(pong[1:9], pingTime[:]))
132132
}
133+
134+
// TestOfflineDiscoveryPingRepliesImmediately guards the LAN-list bug where
135+
// phantom waited on a recovery probe before sending OfflinePong. Blackholed
136+
// remotes take the full deadline, so consoles timed out and never showed the
137+
// offline entry even though a pong was eventually sent.
138+
func TestOfflineDiscoveryPingRepliesImmediately(t *testing.T) {
139+
blackhole, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0})
140+
require.NoError(t, err)
141+
t.Cleanup(func() { _ = blackhole.Close() })
142+
143+
proxyServer, err := New(ProxyPrefs{
144+
BindAddress: "127.0.0.1",
145+
BindPort: 0,
146+
RemoteServer: blackhole.LocalAddr().String(),
147+
IdleTimeout: time.Minute,
148+
NumWorkers: 1,
149+
DisableDiscoveryListener: true,
150+
})
151+
require.NoError(t, err)
152+
153+
dataConn, err := net.ListenUDP("udp", proxyServer.bindAddress)
154+
require.NoError(t, err)
155+
proxyServer.server.Store(dataConn)
156+
t.Cleanup(func() {
157+
proxyServer.dead.Set()
158+
_ = dataConn.Close()
159+
proxyServer.clientMap.Close()
160+
})
161+
162+
// If handleDiscoveryPing waited for recovery, this would delay the reply.
163+
prev := discoveryRecoveryProbeTimeoutNanos.Swap(int64(2 * time.Second))
164+
t.Cleanup(func() { discoveryRecoveryProbeTimeoutNanos.Store(prev) })
165+
166+
proxyServer.markServerOffline()
167+
168+
clientConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0})
169+
require.NoError(t, err)
170+
t.Cleanup(func() { _ = clientConn.Close() })
171+
clientAddr := clientConn.LocalAddr().(*net.UDPAddr)
172+
173+
pingTime := [8]byte{9, 8, 7, 6, 5, 4, 3, 2}
174+
started := time.Now()
175+
require.NoError(t, proxyServer.handleDiscoveryPing(clientAddr, bareUnconnectedPing(pingTime)))
176+
elapsed := time.Since(started)
177+
if elapsed >= 200*time.Millisecond {
178+
t.Fatalf("offline pong took %v; must not wait on upstream recovery probe", elapsed)
179+
}
180+
181+
_ = clientConn.SetReadDeadline(time.Now().Add(time.Second))
182+
buf := make([]byte, 2048)
183+
n, _, err := clientConn.ReadFromUDP(buf)
184+
require.NoError(t, err)
185+
require.Equal(t, proto.UnconnectedPongID, buf[0])
186+
require.True(t, bytes.Equal(buf[1:9], pingTime[:]), "pong must echo ping time")
187+
188+
packet, err := proto.ReadUnconnectedPing(buf[:n])
189+
require.NoError(t, err)
190+
require.Contains(t, packet.Pong.MOTD, "offline")
191+
}

‎internal/proxy/discovery_test.go‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ func TestHandleUnconnectedPingOfflineReply(t *testing.T) {
1818
defer client.Close()
1919

2020
// Closed port: discovery probe fails quickly and offline pong is advertised.
21+
prev := discoveryPingTimeoutNanos.Swap(int64(200 * time.Millisecond))
22+
t.Cleanup(func() { discoveryPingTimeoutNanos.Store(prev) })
23+
2124
p, err := New(ProxyPrefs{
2225
BindAddress: "127.0.0.1",
2326
BindPort: 0,
@@ -34,10 +37,6 @@ func TestHandleUnconnectedPingOfflineReply(t *testing.T) {
3437
}
3538
defer p.Close()
3639

37-
prev := discoveryPingTimeout
38-
discoveryPingTimeout = 200 * time.Millisecond
39-
defer func() { discoveryPingTimeout = prev }()
40-
4140
ping := []byte{proto.UnconnectedPingID, 1, 2, 3, 4, 5, 6, 7, 8}
4241
if err := p.HandleUnconnectedPing(ping, client.LocalAddr()); err != nil {
4342
t.Fatalf("HandleUnconnectedPing: %v", err)
@@ -71,6 +70,9 @@ func TestHandleUnconnectedPingOpenConnections(t *testing.T) {
7170
}
7271
defer client.Close()
7372

73+
prev := discoveryPingTimeoutNanos.Swap(int64(200 * time.Millisecond))
74+
t.Cleanup(func() { discoveryPingTimeoutNanos.Store(prev) })
75+
7476
p, err := New(ProxyPrefs{
7577
BindAddress: "127.0.0.1",
7678
BindPort: 0,
@@ -87,10 +89,6 @@ func TestHandleUnconnectedPingOpenConnections(t *testing.T) {
8789
}
8890
defer p.Close()
8991

90-
prev := discoveryPingTimeout
91-
discoveryPingTimeout = 200 * time.Millisecond
92-
defer func() { discoveryPingTimeout = prev }()
93-
9492
ping := []byte{proto.UnconnectedPingOpenID, 1, 2, 3, 4, 5, 6, 7, 8}
9593
if err := p.HandleUnconnectedPing(ping, client.LocalAddr()); err != nil {
9694
t.Fatalf("HandleUnconnectedPing 0x02: %v", err)

‎internal/proxy/proxy.go‎

Lines changed: 85 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,31 @@ const offlineTimeoutThreshold = 3
3838

3939
var idleCheckInterval = 5 * time.Second
4040

41-
// discoveryPingTimeout bounds how long a LAN probe waits for upstream. Keep this
42-
// below compat PingTimeout (2s) so clients pinging the bind port get a timely reply.
43-
var discoveryPingTimeout = 1500 * time.Millisecond
41+
// Discovery probe deadlines (nanoseconds). Stored atomically so tests can shorten
42+
// them without racing the background health-check goroutine.
43+
var (
44+
discoveryPingTimeoutNanos atomic.Int64
45+
discoveryRecoveryProbeTimeoutNanos atomic.Int64
46+
discoveryHealthIntervalNanos atomic.Int64
47+
)
48+
49+
func init() {
50+
discoveryPingTimeoutNanos.Store(int64(1500 * time.Millisecond))
51+
discoveryRecoveryProbeTimeoutNanos.Store(int64(500 * time.Millisecond))
52+
discoveryHealthIntervalNanos.Store(int64(2 * time.Second))
53+
}
54+
55+
func discoveryPingTimeout() time.Duration {
56+
return time.Duration(discoveryPingTimeoutNanos.Load())
57+
}
58+
59+
func discoveryRecoveryProbeTimeout() time.Duration {
60+
return time.Duration(discoveryRecoveryProbeTimeoutNanos.Load())
61+
}
4462

45-
// discoveryRecoveryProbeTimeout is used once the server is already marked offline:
46-
// a quick check whether upstream came back without making every LAN ping wait out
47-
// the full discoveryPingTimeout.
48-
var discoveryRecoveryProbeTimeout = 500 * time.Millisecond
63+
func discoveryHealthInterval() time.Duration {
64+
return time.Duration(discoveryHealthIntervalNanos.Load())
65+
}
4966

5067
type ProxyServer struct {
5168
bindAddress *net.UDPAddr
@@ -185,6 +202,10 @@ func (proxy *ProxyServer) listen() error {
185202
}
186203
proxy.server.Store(proxyServer)
187204

205+
// Learn offline/online before the first console ping, and recover without
206+
// making OfflinePong wait on a synchronous probe.
207+
proxy.startUpstreamHealthCheck()
208+
188209
return nil
189210
}
190211

@@ -438,6 +459,18 @@ func (proxy *ProxyServer) handleDiscoveryPing(client net.Addr, ping []byte) erro
438459
return fmt.Errorf("proxy not running")
439460
}
440461

462+
// Already offline: reply immediately. Waiting on a recovery probe made
463+
// OfflinePong arrive after console discovery timeouts — blackholed remotes
464+
// take the full deadline, so the LAN entry never appeared even though logs
465+
// later showed a pong was sent.
466+
if proxy.isServerOffline() {
467+
if _, err := server.WriteTo(proxy.buildOfflinePong(ping), client); err != nil {
468+
return err
469+
}
470+
log.Info().Msgf("Sent server offline pong to client: %v", client.String())
471+
return nil
472+
}
473+
441474
pong, err := proxy.probeRemoteUnconnectedPong(ping)
442475
if err != nil {
443476
proxy.markServerOffline()
@@ -458,6 +491,49 @@ func (proxy *ProxyServer) handleDiscoveryPing(client net.Addr, ping []byte) erro
458491
return nil
459492
}
460493

494+
// startUpstreamHealthCheck probes the remote on a timer so OfflinePong can be
495+
// advertised before the first LAN ping, and so recovery does not require a
496+
// client to wait out a synchronous probe.
497+
func (proxy *ProxyServer) startUpstreamHealthCheck() {
498+
go func() {
499+
proxy.probeUpstreamHealth()
500+
ticker := time.NewTicker(discoveryHealthInterval())
501+
defer ticker.Stop()
502+
for {
503+
select {
504+
case <-ticker.C:
505+
if proxy.dead.IsSet() {
506+
return
507+
}
508+
proxy.probeUpstreamHealth()
509+
}
510+
}
511+
}()
512+
}
513+
514+
func (proxy *ProxyServer) probeUpstreamHealth() {
515+
if proxy.dead.IsSet() || proxy.dataConn() == nil {
516+
return
517+
}
518+
_, err := proxy.probeRemoteUnconnectedPong(healthCheckPing())
519+
if err != nil {
520+
proxy.markServerOffline()
521+
return
522+
}
523+
proxy.noteUpstreamReachable()
524+
}
525+
526+
// healthCheckPing is a minimal valid Unconnected Ping for background probes.
527+
func healthCheckPing() []byte {
528+
magic := []byte{0x00, 0xff, 0xff, 0x00, 0xfe, 0xfe, 0xfe, 0xfe, 0xfd, 0xfd, 0xfd, 0xfd, 0x12, 0x34, 0x56, 0x78}
529+
out := make([]byte, 0, 1+8+8+16)
530+
out = append(out, proto.UnconnectedPingID)
531+
out = append(out, 0, 0, 0, 0, 0, 0, 0, 0) // ping time
532+
out = append(out, 0, 0, 0, 0, 0, 0, 0, 0) // client GUID
533+
out = append(out, magic...)
534+
return out
535+
}
536+
461537
// probeRemoteUnconnectedPong dials a one-shot UDP socket to the remote and
462538
// waits for an Unconnected Pong. A fresh local port avoids stale RakNet state.
463539
func (proxy *ProxyServer) probeRemoteUnconnectedPong(ping []byte) ([]byte, error) {
@@ -467,9 +543,9 @@ func (proxy *ProxyServer) probeRemoteUnconnectedPong(ping []byte) ([]byte, error
467543
}
468544
defer conn.Close()
469545

470-
timeout := discoveryPingTimeout
546+
timeout := discoveryPingTimeout()
471547
if proxy.isServerOffline() {
472-
timeout = discoveryRecoveryProbeTimeout
548+
timeout = discoveryRecoveryProbeTimeout()
473549
}
474550
_ = conn.SetDeadline(time.Now().Add(timeout))
475551
if _, err := conn.Write(ping); err != nil {

‎internal/proxy/udp_recv_buffer_test.go‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"net"
66
"testing"
77
"time"
8+
9+
"github.com/jhead/phantom/internal/proto"
810
)
911

1012
func TestUDPRecvBufferCoversOversizedRakNetDatagrams(t *testing.T) {
@@ -60,19 +62,13 @@ func TestProxyForwardsLargeDatagramsBothWays(t *testing.T) {
6062
}
6163
defer remote.Close()
6264

63-
// Exclusive :19132 bind in Start() — skip cleanly if another phantom owns it.
64-
probe, err := net.ListenPacket("udp4", "127.0.0.1:19132")
65-
if err != nil {
66-
t.Skipf("port 19132 unavailable: %v", err)
67-
}
68-
probe.Close()
69-
7065
p, err := New(ProxyPrefs{
71-
BindAddress: "127.0.0.1",
72-
BindPort: 0,
73-
RemoteServer: remote.LocalAddr().String(),
74-
IdleTimeout: time.Minute,
75-
NumWorkers: 1,
66+
BindAddress: "127.0.0.1",
67+
BindPort: 0,
68+
RemoteServer: remote.LocalAddr().String(),
69+
IdleTimeout: time.Minute,
70+
NumWorkers: 1,
71+
DisableDiscoveryListener: true,
7672
})
7773
if err != nil {
7874
t.Fatal(err)
@@ -111,11 +107,21 @@ func TestProxyForwardsLargeDatagramsBothWays(t *testing.T) {
111107
t.Fatalf("client write %d: %v", size, err)
112108
}
113109

114-
_ = remote.SetReadDeadline(time.Now().Add(time.Second))
115110
rbuf := make([]byte, udpRecvBufferSize)
116-
rn, from, err := remote.ReadFrom(rbuf)
117-
if err != nil {
118-
t.Fatalf("remote read %d: %v", size, err)
111+
var rn int
112+
var from net.Addr
113+
readDeadline := time.Now().Add(2 * time.Second)
114+
for {
115+
_ = remote.SetReadDeadline(readDeadline)
116+
rn, from, err = remote.ReadFrom(rbuf)
117+
if err != nil {
118+
t.Fatalf("remote read %d: %v", size, err)
119+
}
120+
// Background health checks send Unconnected Pings; skip them.
121+
if rn >= 1 && proto.IsUnconnectedDiscoveryPing(rbuf[0]) {
122+
continue
123+
}
124+
break
119125
}
120126
if rn != size || !bytes.Equal(rbuf[:rn], up) {
121127
t.Fatalf("upstream got n=%d want %d (truncated or corrupt)", rn, size)

0 commit comments

Comments
 (0)