From 569572d6f73d9d06bb5bbd997b3d8461561f6fb9 Mon Sep 17 00:00:00 2001 From: ginuerzh Date: Sat, 23 May 2026 16:30:53 +0800 Subject: [PATCH] fix(chain): Copy returns wrong pointer, connection leaks in Connect/Handshake, deferred cancel accumulation in retry loops - transport.go: Copy() returned original pointer instead of the allocated copy, causing data races and incorrect routing for multiplexed chains. - route.go: Connect and Handshake failures in intermediate nodes leaked the returned connection. Handshake fix preserves the original Connect result (guaranteed non-nil) and closes both the Handshake return value and input. - router.go: defer cancel() inside retry loops accumulated context timers across iterations. Wrapped each loop body in an IIFE so defer scopes per-iteration and timers are released immediately. --- chain/route.go | 11 ++- chain/router.go | 162 ++++++++++++++++++++++++--------------------- chain/transport.go | 2 +- 3 files changed, 97 insertions(+), 78 deletions(-) diff --git a/chain/route.go b/chain/route.go index 76b7b585..811efba2 100644 --- a/chain/route.go +++ b/chain/route.go @@ -271,20 +271,29 @@ func (r *chainRoute) connect(ctx context.Context, logger logger.Logger) (conn ne } cc, err = preNode.Options().Transport.Connect(ctx, cn, "tcp", addr) if err != nil { + if cc != nil { + cc.Close() + } cn.Close() if marker != nil { marker.Mark() } return } - cc, err = node.Options().Transport.Handshake(ctx, cc) + var cc2 net.Conn + cc2, err = node.Options().Transport.Handshake(ctx, cc) if err != nil { + if cc2 != nil { + cc2.Close() + } + cc.Close() cn.Close() if marker != nil { marker.Mark() } return } + cc = cc2 if marker != nil { marker.Reset() } diff --git a/chain/router.go b/chain/router.go index 3a0317a4..2943d409 100644 --- a/chain/router.go +++ b/chain/router.go @@ -90,58 +90,63 @@ func (r *Router) dial(ctx context.Context, network, address string, log logger.L log.Debugf("dial %s/%s", address, network) for i := 0; i < count; i++ { - ctx := ctx - if r.options.Timeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, r.options.Timeout) - defer cancel() - } + func() { + ctx := ctx + if r.options.Timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, r.options.Timeout) + defer cancel() + } - buf := ictx.BufferFromContext(ctx) - if buf != nil { - buf.Reset() - } + buf := ictx.BufferFromContext(ctx) + if buf != nil { + buf.Reset() + } - var ipAddr string - ipAddr, err = xnet.Resolve(ctx, "ip", address, r.options.Resolver, r.options.HostMapper, log) - if err != nil { - log.Error(err) + var ipAddr string + ipAddr, err = xnet.Resolve(ctx, "ip", address, r.options.Resolver, r.options.HostMapper, log) + if err != nil { + log.Error(err) + return + } + + if buf != nil { + buf.Reset() + } + + var route chain.Route + if r.options.Chain != nil { + route = r.options.Chain.Route(ctx, network, ipAddr, chain.WithHostRouteOption(address)) + } + + if buf == nil { + buf = &bytes.Buffer{} + } + for _, node := range routePath(route) { + fmt.Fprintf(buf, "%s@%s > ", node.Name, node.Addr) + } + fmt.Fprintf(buf, "%s", ipAddr) + log.Debugf("route(retry=%d) %s", i, buf.String()) + + if route == nil { + route = DefaultRoute + } + conn, err = route.Dial(ctx, network, ipAddr, + append([]chain.DialOption{ + chain.InterfaceDialOption(r.options.IfceName), + chain.NetnsDialOption(r.options.Netns), + chain.SockOptsDialOption(r.options.SockOpts), + chain.LoggerDialOption(log), + }, callerOpts...)..., + ) + if err == nil { + return + } + log.Errorf("route(retry=%d) %s", i, err) + }() + if conn != nil || err == nil { break } - - if buf != nil { - buf.Reset() - } - - var route chain.Route - if r.options.Chain != nil { - route = r.options.Chain.Route(ctx, network, ipAddr, chain.WithHostRouteOption(address)) - } - - if buf == nil { - buf = &bytes.Buffer{} - } - for _, node := range routePath(route) { - fmt.Fprintf(buf, "%s@%s > ", node.Name, node.Addr) - } - fmt.Fprintf(buf, "%s", ipAddr) - log.Debugf("route(retry=%d) %s", i, buf.String()) - - if route == nil { - route = DefaultRoute - } - conn, err = route.Dial(ctx, network, ipAddr, - append([]chain.DialOption{ - chain.InterfaceDialOption(r.options.IfceName), - chain.NetnsDialOption(r.options.Netns), - chain.SockOptsDialOption(r.options.SockOpts), - chain.LoggerDialOption(log), - }, callerOpts...)..., - ) - if err == nil { - break - } - log.Errorf("route(retry=%d) %s", i, err) } return @@ -160,39 +165,44 @@ func (r *Router) Bind(ctx context.Context, network, address string, opts ...chai log.Debugf("bind on %s/%s", address, network) for i := 0; i < count; i++ { - ctx := ctx - if r.options.Timeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, r.options.Timeout) - defer cancel() - } + func() { + ctx := ctx + if r.options.Timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, r.options.Timeout) + defer cancel() + } - var route chain.Route - if r.options.Chain != nil { - route = r.options.Chain.Route(ctx, network, address) - if route == nil || len(route.Nodes()) == 0 { - err = ErrEmptyRoute + var route chain.Route + if r.options.Chain != nil { + route = r.options.Chain.Route(ctx, network, address) + if route == nil || len(route.Nodes()) == 0 { + err = ErrEmptyRoute + return + } + } + + if log.IsLevelEnabled(logger.DebugLevel) { + buf := bytes.Buffer{} + for _, node := range routePath(route) { + fmt.Fprintf(&buf, "%s@%s > ", node.Name, node.Addr) + } + fmt.Fprintf(&buf, "%s", address) + log.Debugf("route(retry=%d) %s", i, buf.String()) + } + + if route == nil { + route = DefaultRoute + } + ln, err = route.Bind(ctx, network, address, opts...) + if err == nil { return } - } - - if log.IsLevelEnabled(logger.DebugLevel) { - buf := bytes.Buffer{} - for _, node := range routePath(route) { - fmt.Fprintf(&buf, "%s@%s > ", node.Name, node.Addr) - } - fmt.Fprintf(&buf, "%s", address) - log.Debugf("route(retry=%d) %s", i, buf.String()) - } - - if route == nil { - route = DefaultRoute - } - ln, err = route.Bind(ctx, network, address, opts...) - if err == nil { + log.Errorf("route(retry=%d) %s", i, err) + }() + if ln != nil || err == nil { break } - log.Errorf("route(retry=%d) %s", i, err) } return diff --git a/chain/transport.go b/chain/transport.go index ed999f42..a1db527d 100644 --- a/chain/transport.go +++ b/chain/transport.go @@ -102,5 +102,5 @@ func (tr *Transport) Options() *chain.TransportOptions { func (tr *Transport) Copy() chain.Transporter { tr2 := &Transport{} *tr2 = *tr - return tr + return tr2 }