From 469106d50279d15ead6c9be93e2dd3c77e59510f Mon Sep 17 00:00:00 2001 From: kevin Date: Thu, 9 Jul 2026 23:19:46 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8DraceDial=E7=AB=9E?= =?UTF-8?q?=E6=80=81=E6=9D=A1=E4=BB=B6=E5=AF=BC=E8=87=B4daemon=E6=AD=BB?= =?UTF-8?q?=E9=94=81=E6=97=A0=E6=B3=95=E8=BF=9E=E6=8E=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit raceDial在首个连接成功后cancel剩余拨号goroutine时，dial goroutine 的select有两个同时就绪的case（缓冲channel可发送 vs raceCtx.Done()）， Go随机选择可能导致结果未发送，使drain循环永久阻塞，进而导致daemon 持有d.mu死锁，所有IPC命令(stats/stop/新start)全部阻塞。 DNS解析返回IPv4+IPv6双地址时触发此bug，约50%概率死锁。 修复：移除select，直接发送到缓冲channel（容量=len(ips)永不阻塞）， 多余连接由drain循环负责关闭。 --- internal/transport/racedial.go | 17 ++-- internal/transport/racedial_test.go | 133 ++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 8 deletions(-) create mode 100644 internal/transport/racedial_test.go diff --git a/internal/transport/racedial.go b/internal/transport/racedial.go index 1cde120..43c293d 100644 --- a/internal/transport/racedial.go +++ b/internal/transport/racedial.go @@ -99,13 +99,13 @@ func raceDial(ctx context.Context, network string, ips []string, port string) (n go func(target string) { d := &net.Dialer{} c, err := d.DialContext(raceCtx, network, net.JoinHostPort(target, port)) - select { - case resultCh <- result{conn: c, err: err}: - case <-raceCtx.Done(): - if c != nil { - c.Close() - } - } + // Always send the result: resultCh is buffered (len(ips)), + // so this never blocks. Do NOT use a select with + // <-raceCtx.Done() here - when the context is cancelled + // both cases would be ready and Go's random select pick + // could skip the send, causing the drain loop below to + // block forever. + resultCh <- result{conn: c, err: err} }(ip) } @@ -117,7 +117,8 @@ func raceDial(ctx context.Context, network string, ips []string, port string) (n // any late successful connections. cancel() for j := i + 1; j < len(ips); j++ { - if late := <-resultCh; late.conn != nil { + late := <-resultCh + if late.conn != nil { late.conn.Close() } } diff --git a/internal/transport/racedial_test.go b/internal/transport/racedial_test.go new file mode 100644 index 0000000..db832a8 --- /dev/null +++ b/internal/transport/racedial_test.go @@ -0,0 +1,133 @@ +package transport + +import ( + "context" + "net" + "sync" + "testing" + "time" +) + +// TestRaceDialNoDeadlock is a regression test for a select race +// condition that caused raceDial to deadlock ~50% of the time when +// multiple IPs were raced and the first succeeded. The bug was a +// select with two simultaneously-ready cases (send result vs +// <-raceCtx.Done()); Go's random selection could skip the send, +// leaving the drain loop blocked forever. +// +// We run many iterations because the bug was probabilistic. +func TestRaceDialNoDeadlock(t *testing.T) { + const iterations = 200 + + for n := 0; n < iterations; n++ { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + + // Accept both connections in background (both IPs dial the + // same listener since raceDial uses a single port). + go func() { + for { + c, err := ln.Accept() + if err != nil { + return + } + c.Close() + } + }() + + port := portOf(ln.Addr()) + ips := []string{"127.0.0.1", "127.0.0.1"} + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + conn, err := raceDial(ctx, "tcp", ips, port) + cancel() + if err == nil && conn != nil { + conn.Close() + } + + ln.Close() + + // If we get here without the 5s timeout, the iteration passed. + // A deadlock would trigger the test-wide 60s timeout. + } + + // If we reach here, no iteration deadlocked. +} + +// TestRaceDialAllFail verifies that when all dials fail, raceDial +// returns an error instead of blocking. +func TestRaceDialAllFail(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // Port 1: connection refused on most systems. + ips := []string{"127.0.0.1", "127.0.0.1"} + _, err := raceDial(ctx, "tcp", ips, "1") + if err == nil { + t.Fatal("expected error when all dials fail") + } +} + +// TestRaceDialSingleIP verifies the single-IP path still works. +func TestRaceDialSingleIP(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + c, _ := ln.Accept() + if c != nil { + c.Close() + } + }() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + conn, err := raceDial(ctx, "tcp", []string{"127.0.0.1"}, portOf(ln.Addr())) + if err != nil { + t.Fatalf("raceDial single IP: %v", err) + } + conn.Close() + wg.Wait() +} + +// TestRaceDialContextCancelled ensures raceDial returns promptly when +// the parent context is cancelled while dials are in flight. +func TestRaceDialContextCancelled(t *testing.T) { + // Dial a non-routable address so the dial hangs until context + // cancellation. + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(200 * time.Millisecond) + cancel() + }() + + done := make(chan error, 1) + go func() { + _, err := raceDial(ctx, "tcp", []string{"10.255.255.1", "10.255.255.2"}, "80") + done <- err + }() + + select { + case err := <-done: + if err == nil { + t.Fatal("expected error on cancelled context") + } + case <-time.After(5 * time.Second): + t.Fatal("raceDial did not return after context cancellation") + } +} + +// portOf extracts the port from a net.Addr. +func portOf(addr net.Addr) string { + _, port, _ := net.SplitHostPort(addr.String()) + return port +}