nuts-foundation / nuts-foundation/nuts-node
Race condition: inbound connections never call disconnect(), can wedge reconnects and gossip delivery
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 28
- Forks
- 23
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 76
Description
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 callsconnection.disconnect()as soon as any protocol stream closes. -
Inbound (
handleInboundStream, line ~582-644) only removes the connection from the list, it never callsconnection.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 returningtruefor the dead connection, becausemc.streamsis never cleared.- The peer's
ID/NodeDIDare not reset, so the dead connection still matches onByPeerID/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:
getOrRegisterfinds the stale, not-yet-removed connection and reuses it (created == false).registerStream(connection.go:193-215) checksmc.streams[methodName] != nil— since the old (dead) stream was never cleared, this is still true, so the new stream registration is rejected andhandleInboundStreamreturnsErrAlreadyConnected, even though the "existing" connection is actually dead.- Independently, because
mc.outboxesfor the dead connection is also never closed, anything still trying toSend()on that staleConnection(e.g. gossip) writes into a channel that nobody is draining anymore (thestartSendinggoroutine already exited whenmc.ctxwas cancelled) — messages are silently dropped instead of surfacingErrNoConnection.
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
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in network/transport/grpc/connection_manager.go at handleInboundStream and compare its teardown with connect and openOutboundStreams. Trace connection.disconnect() in connection.go and inbound matching in connection_list.go, then inspect the v2 gossip lookup in network/transport/v2/protocol.go. Done means inbound teardown clears streams, outboxes, and peer identity before removal so a fast reconnect is not rejected or shadowed by stale state.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100