Race condition: inbound connections never call disconnect(), can wedge reconnects and gossip delivery

[reinkrul]

Observed symptom

Production logs show a peer repeatedly failing to receive gossip messages for extended periods, without any accompanying reconnect/disconnect log entries for that peer in between:

time="2026-09-03T07:58:26Z" level=info msg="Peer closed connection" module=Network peerAddr=46.101.96.139 peerAuthenticated=false peerDID= peerID=237fa970-0832-4540-869a-b6adf57b341f protocolVersion=2

time="2026-09-03T08:00:05Z" level=error msg="failed to send Gossip message" error="no connection available" module=Network peerID=b373e316-0884-4dc4-8760-1b6234956b17

time="2026-09-03T08:05:25Z" level=error msg="failed to send Gossip message" error="no connection available" module=Network peerID=b373e316-0884-4dc4-8760-1b6234956b17

(Different peerIDs in these three lines — the Peer closed connection line is not necessarily the direct cause of the gossip failures, but the pattern of a peer being stuck in a state where ConnectionList.Get(ByConnected(), ByPeer(...)) keeps returning nothing, for minutes at a time, points at a connection bookkeeping bug rather than a simple "peer is offline" situation.)

Root cause: asymmetry between inbound and outbound teardown

network/transport/grpc/connection_manager.go:

• Outbound (connect, line ~291-303) always tears the connection down fully on exit:

  defer func() {
      connection.disconnect()
      s.connections.remove(connection)
  }()

  and openOutboundStreams (line ~446-451) additionally calls connection.disconnect() as soon as any protocol stream closes.

• Inbound (handleInboundStream, line ~582-644) only removes the connection from the list, it never calls connection.disconnect():

  s.notifyObservers(peer, protocol, transport.StateConnected)
  connection.waitUntilDisconnected()
  s.notifyObservers(peer, protocol, transport.StateDisconnected)
  
  s.connections.remove(connection)
  return nil

Connection.disconnect() (network/transport/grpc/connection.go:122-143) is what clears mc.streams/mc.outboxes and resets the peer's ID/NodeDID/Authenticated fields. Skipping it for inbound connections leaves the conn object in a stale, half-torn-down state:

• IsConnected() (len(mc.streams) > 0) keeps returning true for the dead connection, because mc.streams is never cleared.
• The peer's ID/NodeDID are not reset, so the dead connection still matches on ByPeerID/ByNodeDID.

Race window

getOrRegister for inbound connections (connection_list.go:101-106) matches purely on ByPeerID(peer.ID), ByNodeDID(peer.NodeDID) (no address component). Between the moment a stream is detected as closed (mc.cancelCtx() in connection.go:startReceiving/registerStream) and the moment s.connections.remove(connection) actually runs in handleInboundStream, the dead connection is still present in connectionList.list and still reports IsConnected() == true with its old peer identity intact.

If the same peer reconnects inbound inside that window:

• getOrRegister finds the stale, not-yet-removed connection and reuses it (created == false).
• registerStream (connection.go:193-215) checks mc.streams[methodName] != nil — since the old (dead) stream was never cleared, this is still true, so the new stream registration is rejected and handleInboundStream returns ErrAlreadyConnected, even though the "existing" connection is actually dead.
• Independently, because mc.outboxes for the dead connection is also never closed, anything still trying to Send() on that stale Connection (e.g. gossip) writes into a channel that nobody is draining anymore (the startSending goroutine already exited when mc.ctx was cancelled) — messages are silently dropped instead of surfacing ErrNoConnection.

Both effects are consistent with a peer appearing intermittently unreachable for gossip (network/transport/v2/protocol.go:238-258, via connectionList.Get(grpc.ByConnected(), grpc.ByPeer(...))) well after the underlying TCP/gRPC connection is actually gone, without a clean, timely reconnect.

Suggested fix

Make handleInboundStream symmetric with the outbound path: call connection.disconnect() before (or as part of) s.connections.remove(connection), e.g.:

connection.waitUntilDisconnected()
s.notifyObservers(peer, protocol, transport.StateDisconnected)

connection.disconnect()
s.connections.remove(connection)

This ensures the connection's streams/outboxes are cleared and its peer identity is reset as soon as it's torn down, closing the race window where a stale, "connected"-looking Connection can shadow the peer during a fast reconnect.

Environment

• Observed on a V6.2-based deployment (network module, protocol v2, gRPC transport).

Assisted by AI