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

未关闭
#4,468 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
3/5
预计耗时
1-2 天
新手友好度
74/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃
技术栈
go
领域
networking

调研方向

从 network/transport/grpc/connection_manager.go 中的 handleInboundStream 开始,将其 teardown 与 connect 和 openOutboundStreams 进行比较。跟踪 connection.go 中的 connection.disconnect() 以及 connection_list.go 中的入站匹配,然后检查 network/transport/v2/protocol.go 中的 v2 gossip lookup。完成标准是:入站 teardown 在移除之前清理 streams、outboxes 和 peer identity,从而使快速重连不会被拒绝,也不会被陈旧状态掩盖。

由索引模型根据 Issue 内容生成。

描述

bug

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

主要语言
Go
星标
29
派生
24
平均合并
1 天 9 小时
30 天内合并 PR
48

环境准备

  • 提供 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 没有贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

nuts-foundation/nuts-node 的其他 Issue

查看 nuts-foundation/nuts-node 的全部 Issue

相似的 Issue

更多 Go Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。