diff --git a/client/system/process.go b/client/system/process.go index 07f69a212..fefa7d913 100644 --- a/client/system/process.go +++ b/client/system/process.go @@ -7,7 +7,7 @@ import ( "os" "slices" - "github.com/shirou/gopsutil/v3/process" + "github.com/shirou/gopsutil/v4/process" ) // getRunningProcesses returns a list of running process paths. The context bounds the work: diff --git a/client/system/process_test.go b/client/system/process_test.go index 44a1c8ba0..9d0a6b935 100644 --- a/client/system/process_test.go +++ b/client/system/process_test.go @@ -4,7 +4,7 @@ import ( "context" "testing" - "github.com/shirou/gopsutil/v3/process" + "github.com/shirou/gopsutil/v4/process" ) func Benchmark_getRunningProcesses(b *testing.B) { diff --git a/go.mod b/go.mod index 3f1f8e414..dbbd3e35b 100644 --- a/go.mod +++ b/go.mod @@ -101,7 +101,7 @@ require ( github.com/quic-go/quic-go v0.55.0 github.com/redis/go-redis/v9 v9.7.3 github.com/rs/xid v1.3.0 - github.com/shirou/gopsutil/v3 v3.24.4 + github.com/shirou/gopsutil/v4 v4.25.8 github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 github.com/songgao/water v0.0.0-20200317203138-2b4b6d7c09d8 github.com/stretchr/testify v1.11.1 @@ -295,8 +295,6 @@ require ( github.com/prometheus/procfs v0.19.2 // indirect github.com/russellhaering/goxmldsig v1.6.0 // indirect github.com/ryanuber/go-glob v1.0.0 // indirect - github.com/shirou/gopsutil/v4 v4.25.8 // indirect - github.com/shoenig/go-m1cpu v0.2.1 // indirect github.com/shopspring/decimal v1.4.0 // indirect github.com/spf13/cast v1.10.0 // indirect github.com/stretchr/objx v0.5.2 // indirect diff --git a/go.sum b/go.sum index b2aa6cddd..d4be37df0 100644 --- a/go.sum +++ b/go.sum @@ -271,9 +271,7 @@ github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= @@ -417,7 +415,6 @@ github.com/libp2p/go-netroute v0.4.0 h1:sZZx9hyANYUx9PZyqcgE/E1GUG3iEtTZHUEvdtXT github.com/libp2p/go-netroute v0.4.0/go.mod h1:Nkd5ShYgSMS5MUKy/MU2T57xFoOKvvLR92Lic48LEyA= github.com/lrh3321/ipset-go v0.0.0-20250619021614-54a0a98ace81 h1:J56rFEfUTFT9j9CiRXhi1r8lUJ4W5idG3CiaBZGojNU= github.com/lrh3321/ipset-go v0.0.0-20250619021614-54a0a98ace81/go.mod h1:RD8ML/YdXctQ7qbcizZkw5mZ6l8Ogrl1dodBzVJduwI= -github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I= github.com/lufia/plan9stats v0.0.0-20240513124658-fba389f38bae h1:dIZY4ULFcto4tAFlj1FYZl8ztUZ13bdq+PLY+NOfbyI= github.com/lufia/plan9stats v0.0.0-20240513124658-fba389f38bae/go.mod h1:ilwx/Dta8jXAgpFYFvSWEMwxmbWXyiUHkd5FwyKhb5k= github.com/magiconair/properties v1.8.10 h1:s31yESBquKXCV9a/ScB3ESkOjUYYv+X0rg8SYxI99mE= @@ -569,7 +566,6 @@ github.com/pkg/sftp v1.13.9/go.mod h1:OBN7bVXdstkFFN/gdnHPUb5TE8eb8G1Rp9wCItqjkk github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE= github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 h1:o4JXh1EVt9k/+g42oCprj/FisM4qX9L3sZB3upGN2ZU= github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE= github.com/pquerna/otp v1.5.0 h1:NMMR+WrmaqXU4EzdGJEE1aUUI0AMRzsp96fFFWNPwxs= @@ -599,16 +595,8 @@ github.com/russellhaering/goxmldsig v1.6.0/go.mod h1:TrnaquDcYxWXfJrOjeMBTX4mLBe github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc= -github.com/shirou/gopsutil/v3 v3.24.4 h1:dEHgzZXt4LMNm+oYELpzl9YCqV65Yr/6SfrvgRBtXeU= -github.com/shirou/gopsutil/v3 v3.24.4/go.mod h1:lTd2mdiOspcqLgAnr9/nGi71NkeMpWKdmhuxm9GusH8= github.com/shirou/gopsutil/v4 v4.25.8 h1:NnAsw9lN7587WHxjJA9ryDnqhJpFH6A+wagYWTOH970= github.com/shirou/gopsutil/v4 v4.25.8/go.mod h1:q9QdMmfAOVIw7a+eF86P7ISEU6ka+NLgkUxlopV4RwI= -github.com/shoenig/go-m1cpu v0.1.6/go.mod h1:1JJMcUBvfNwpq05QDQVAnx3gUHr9IYF7GNg9SUEw2VQ= -github.com/shoenig/go-m1cpu v0.2.1 h1:yqRB4fvOge2+FyRXFkXqsyMoqPazv14Yyy+iyccT2E4= -github.com/shoenig/go-m1cpu v0.2.1/go.mod h1:KkDOw6m3ZJQAPHbrzkZki4hnx+pDRR1Lo+ldA56wD5w= -github.com/shoenig/test v0.6.4/go.mod h1:byHiCGXqrVaflBLAMq/srcZIHynQPQgeyvkvXnjqq0k= -github.com/shoenig/test v1.7.0 h1:eWcHtTXa6QLnBvm0jgEabMRN/uJ4DMV3M8xUGgRkZmk= -github.com/shoenig/test v1.7.0/go.mod h1:UxJ6u/x2v/TNs/LoLxBNJRV9DiwBBKYxXSyczsBHFoI= github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp81k= github.com/shopspring/decimal v1.4.0/go.mod h1:gawqmDU56v4yIKSwfBSFip1HdCCXN8/+DMd9qYNcwME= github.com/sirupsen/logrus v1.9.4 h1:TsZE7l11zFCLZnZ+teH4Umoq5BhEIfIzfRDZ1Uzql2w= @@ -640,7 +628,6 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/testcontainers/testcontainers-go v0.37.0 h1:L2Qc0vkTw2EHWQ08djon0D2uw7Z/PtHS/QzZZ5Ra/hg= @@ -659,10 +646,8 @@ github.com/ti-mo/netfilter v0.5.2 h1:CTjOwFuNNeZ9QPdRXt1MZFLFUf84cKtiQutNauHWd40 github.com/ti-mo/netfilter v0.5.2/go.mod h1:Btx3AtFiOVdHReTDmP9AE+hlkOcvIy403u7BXXbWZKo= github.com/tinylib/msgp v1.6.3 h1:bCSxiTz386UTgyT1i0MSCvdbWjVW+8sG3PjkGsZQt4s= github.com/tinylib/msgp v1.6.3/go.mod h1:RSp0LW9oSxFut3KzESt5Voq4GVWyS+PSulT77roAqEA= -github.com/tklauser/go-sysconf v0.3.12/go.mod h1:Ho14jnntGE1fpdOqQEEaiKRpvIavV0hSfmBq8nJbHYI= github.com/tklauser/go-sysconf v0.3.15 h1:VE89k0criAymJ/Os65CSn1IXaol+1wrsFHEB8Ol49K4= github.com/tklauser/go-sysconf v0.3.15/go.mod h1:Dmjwr6tYFIseJw7a3dRLJfsHAMXZ3nEnL/aZY+0IuI4= -github.com/tklauser/numcpus v0.6.1/go.mod h1:1XfjsgE2zo8GVw7POkMbHENHzVg3GzmoZ9fESEdAacY= github.com/tklauser/numcpus v0.10.0 h1:18njr6LDBk1zuna922MgdjQuJFjrdppsZG60sHGfjso= github.com/tklauser/numcpus v0.10.0/go.mod h1:BiTKazU708GQTYF4mB+cmlpT2Is1gLk7XVuEeem8LsQ= github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVMw= @@ -834,7 +819,6 @@ golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= diff --git a/shared/relay/client/manager.go b/shared/relay/client/manager.go index e1515401e..2f2839d94 100644 --- a/shared/relay/client/manager.go +++ b/shared/relay/client/manager.go @@ -30,11 +30,16 @@ type RelayTrack struct { relayClient *Client err error created time.Time + // ready is closed once the dial started by openConnVia finishes (relayClient + // or err is set). Callers reusing a track wait on this instead of the track + // lock, so the dial never runs under rt.Lock. + ready chan struct{} } func NewRelayTrack() *RelayTrack { return &RelayTrack{ created: time.Now(), + ready: make(chan struct{}), } } @@ -326,34 +331,24 @@ func (m *Manager) openConnVia(ctx context.Context, serverAddress, peerKey string // check if already has a connection to the desired relay server m.relayClientsMutex.RLock() rt, ok := m.relayClients[serverAddress] - if ok { - rt.RLock() - m.relayClientsMutex.RUnlock() - defer rt.RUnlock() - if rt.err != nil { - return nil, rt.err - } - return rt.relayClient.OpenConn(ctx, peerKey) - } m.relayClientsMutex.RUnlock() + if ok { + return m.openConnOnTrack(ctx, rt, peerKey) + } // if not, establish a new connection but check it again (because changed the lock type) before starting the // connection m.relayClientsMutex.Lock() rt, ok = m.relayClients[serverAddress] if ok { - rt.RLock() m.relayClientsMutex.Unlock() - defer rt.RUnlock() - if rt.err != nil { - return nil, rt.err - } - return rt.relayClient.OpenConn(ctx, peerKey) + return m.openConnOnTrack(ctx, rt, peerKey) } - // create a new relay client and store it in the relayClients map + // Publish the track and release the map lock BEFORE dialing, so the dial does + // not run under rt.Lock (which would block RelayStates and the cleanup loop + // for the full dial). Concurrent callers find this track and wait on rt.ready. rt = NewRelayTrack() - rt.Lock() m.relayClients[serverAddress] = rt m.relayClientsMutex.Unlock() @@ -361,8 +356,10 @@ func (m *Manager) openConnVia(ctx context.Context, serverAddress, peerKey string relayClient.SetTransportFallback(m.transportFallback) err := relayClient.Connect(m.ctx) if err != nil { + rt.Lock() rt.err = err rt.Unlock() + close(rt.ready) m.relayClientsMutex.Lock() delete(m.relayClients, serverAddress) m.relayClientsMutex.Unlock() @@ -370,14 +367,34 @@ func (m *Manager) openConnVia(ctx context.Context, serverAddress, peerKey string } // if connection closed then delete the relay client from the list relayClient.SetOnDisconnectListener(m.onServerDisconnected) + rt.Lock() rt.relayClient = relayClient rt.Unlock() + close(rt.ready) - conn, err := relayClient.OpenConn(ctx, peerKey) - if err != nil { - return nil, err + return relayClient.OpenConn(ctx, peerKey) +} + +// openConnOnTrack opens a peer connection through an existing relay track, +// waiting for the dial started by another openConnVia call to finish. It waits +// on rt.ready rather than the track lock, so it neither holds nor contends the +// track lock across the dial. +func (m *Manager) openConnOnTrack(ctx context.Context, rt *RelayTrack, peerKey string) (net.Conn, error) { + select { + case <-rt.ready: + case <-ctx.Done(): + return nil, ctx.Err() } - return conn, nil + + rt.RLock() + defer rt.RUnlock() + if rt.err != nil { + return nil, rt.err + } + if rt.relayClient == nil { + return nil, ErrRelayClientNotConnected + } + return rt.relayClient.OpenConn(ctx, peerKey) } func (m *Manager) onServerConnected() { @@ -476,6 +493,13 @@ func (m *Manager) cleanUpUnusedRelays() { continue } + // dial still in progress (openConnVia publishes the track before Connect + // completes and no longer holds rt.Lock during it), nothing to clean up. + if rt.relayClient == nil { + rt.Unlock() + continue + } + if time.Since(rt.created) <= m.keepUnusedServerTime { rt.Unlock() continue diff --git a/shared/relay/client/manager_cleanup_test.go b/shared/relay/client/manager_cleanup_test.go new file mode 100644 index 000000000..6ac5daeac --- /dev/null +++ b/shared/relay/client/manager_cleanup_test.go @@ -0,0 +1,60 @@ +package client + +import ( + "context" + "net/netip" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// TestCleanUpUnusedRelays_DoesNotBlockOnRealHangingDial drives a real, hanging foreign +// relay dial and asserts cleanUpUnusedRelays does not stall behind it. +func TestCleanUpUnusedRelays_DoesNotBlockOnRealHangingDial(t *testing.T) { + serverAddr := stallingRelayListener(t) + + mCtx, mCancel := context.WithCancel(context.Background()) + t.Cleanup(mCancel) + + m := NewManager(mCtx, nil, "alice", 1280) + + dialDone := make(chan struct{}) + go func() { + defer close(dialDone) + _, _ = m.openConnVia(mCtx, serverAddr, "peerKey", netip.Addr{}) + }() + + // The track appears in the map once the dial is in flight. + require.Eventually(t, func() bool { + m.relayClientsMutex.RLock() + defer m.relayClientsMutex.RUnlock() + _, ok := m.relayClients[serverAddr] + return ok + }, 5*time.Second, 5*time.Millisecond, "relay dial did not start") + + cleanupDone := make(chan struct{}) + go func() { + defer close(cleanupDone) + m.cleanUpUnusedRelays() + }() + + select { + case <-cleanupDone: + case <-time.After(2 * time.Second): + t.Fatal("cleanUpUnusedRelays blocked on an in-progress relay dial while holding the relay map lock") + } + + m.relayClientsMutex.RLock() + _, stillTracked := m.relayClients[serverAddr] + m.relayClientsMutex.RUnlock() + require.True(t, stillTracked, "an in-progress relay dial must not be evicted by cleanup") + + // Release the hanging dial so the goroutine can exit cleanly. + mCancel() + select { + case <-dialDone: + case <-time.After(5 * time.Second): + t.Fatal("openConnVia did not return after context cancellation") + } +} diff --git a/shared/relay/client/manager_relaystates_test.go b/shared/relay/client/manager_relaystates_test.go new file mode 100644 index 000000000..f26323323 --- /dev/null +++ b/shared/relay/client/manager_relaystates_test.go @@ -0,0 +1,91 @@ +package client + +import ( + "context" + "net" + "net/netip" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// stallingRelayListener accepts TCP connections and holds them open without ever +// responding, so a relay handshake dialed against it blocks until its context is +// cancelled. It returns the "rel://host:port" URL to dial. +func stallingRelayListener(t *testing.T) string { + t.Helper() + + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + + var mu sync.Mutex + var conns []net.Conn + go func() { + for { + c, err := ln.Accept() + if err != nil { + return + } + mu.Lock() + conns = append(conns, c) + mu.Unlock() + } + }() + t.Cleanup(func() { + _ = ln.Close() + mu.Lock() + for _, c := range conns { + _ = c.Close() + } + mu.Unlock() + }) + + return "rel://" + ln.Addr().String() +} + +// TestRelayStates_DoesNotBlockOnRealHangingDial is a regression test for +// RelayStates() called by a "status -d command" hanging behind an in-progress +// relay dial. +func TestRelayStates_DoesNotBlockOnRealHangingDial(t *testing.T) { + serverAddr := stallingRelayListener(t) + + mCtx, mCancel := context.WithCancel(context.Background()) + t.Cleanup(mCancel) + + m := NewManager(mCtx, nil, "alice", 1280) + + dialDone := make(chan struct{}) + go func() { + defer close(dialDone) + _, _ = m.openConnVia(mCtx, serverAddr, "peerKey", netip.Addr{}) + }() + + require.Eventually(t, func() bool { + m.relayClientsMutex.RLock() + defer m.relayClientsMutex.RUnlock() + _, ok := m.relayClients[serverAddr] + return ok + }, 5*time.Second, 5*time.Millisecond, "relay dial did not start") + + done := make(chan []RelayConnState, 1) + go func() { + done <- m.RelayStates() + }() + + select { + case states := <-done: + require.Empty(t, states, "a relay still being dialed carries no state and must be omitted") + case <-time.After(2 * time.Second): + t.Fatal("RelayStates blocked on a foreign relay whose Connect() is in progress") + } + + // Release the hanging dial so the goroutine can exit cleanly. + mCancel() + select { + case <-dialDone: + case <-time.After(5 * time.Second): + t.Fatal("openConnVia did not return after context cancellation") + } +}