Skip to content

Commit fbd0df5

Browse files
committed
client/conn_state_pusher: nil-receiver safe On* entry points
Fixes a panic on Engine.Stop teardown: Engine.Stop() e.connStatePusher.Stop() e.connStatePusher = nil ← nilled here ... e.removeAllPeers() ← still triggers Conn.Close UpdatePeerState ← fires status listener listener-closure e.connStatePusher. ← nil receiver OnPeerStateChange() ← deref panic at p.events <- ev Test_Engine_SSH on the netbirdio CI runners hit this reliably because the runners can actually create the wg interface (sandbox cannot, so the test fails earlier locally and never reaches the Stop path). Stack: client/internal/conn_state_pusher.go:99 → engine.go:633 → peer/status.go:300 (notify closure) → peer/conn.go:736 → conn.go:329 → conn_mgr.go:454 → engine.go:864 → engine.go:852 → engine.go:391. Defending in the pusher itself (vs. nil-checking each callsite) keeps the engine cleanup path simple and matches the no-op-on-nil pattern already used elsewhere in the package.
1 parent 7eb350d commit fbd0df5

1 file changed

Lines changed: 14 additions & 1 deletion

File tree

client/internal/conn_state_pusher.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,16 @@ func (p *connStatePusher) Stop() {
9494

9595
// OnPeerStateChange enqueues a state-change event. Non-blocking — drops
9696
// if the buffer is full (the next bulk tick will catch up via delta).
97+
//
98+
// Safe on a nil receiver: Engine.Stop nils e.connStatePusher before
99+
// removeAllPeers runs, but the status-recorder listener registered in
100+
// Engine.Start is still wired and may fire a few more events during
101+
// peer cleanup. A nil-receiver no-op makes the cleanup path cheap and
102+
// avoids a panic on the engine shutdown race.
97103
func (p *connStatePusher) OnPeerStateChange(ev PeerStateChangeEvent) {
104+
if p == nil {
105+
return
106+
}
98107
select {
99108
case p.events <- ev:
100109
default:
@@ -103,8 +112,12 @@ func (p *connStatePusher) OnPeerStateChange(ev PeerStateChangeEvent) {
103112

104113
// OnSnapshotRequest enqueues a snapshot-request nonce. Non-blocking,
105114
// coalescing — multiple requests in flight result in a single full
106-
// snapshot with the latest nonce echoed.
115+
// snapshot with the latest nonce echoed. Nil-receiver safe for the
116+
// same shutdown-race reason as OnPeerStateChange.
107117
func (p *connStatePusher) OnSnapshotRequest(nonce uint64) {
118+
if p == nil {
119+
return
120+
}
108121
select {
109122
case p.snapshotReq <- nonce:
110123
default:

0 commit comments

Comments
 (0)