sendnrw/netbird
2
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-07-18 12:39:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use rt.TryRLock in RelayStates for RelayTrack

Browse Source
This commit is contained in:
Theodor S. Midtlien
2026-07-07 17:57:59 +02:00
parent 59482e5d3d
commit 705ed0566f
2 changed files with 18 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
shared/relay/client/manager.go
View File

@@ -294,8 +294,17 @@ func (m *Manager) RelayStates() []RelayConnState {
// Only connected foreign relays carry state; a failed connect is evicted
// immediately (openConnVia), so there is no error state to surface.
//
// Query each track without blocking: openConnVia holds a track's write-lock
// for the whole of relayClient.Connect() (the network dial). A blocking
// RLock here would stall the status path (GetFullStatus -> GetRelayStates)
// for the full dial timeout. A track mid-Connect has no relayClient set yet
// and would be skipped. TryRLock + skip preserves the result while keeping
// status responsive.
for _, rt := range tracks {
rt.RLock()
if !rt.TryRLock() {
continue
}
rc := rt.relayClient
rt.RUnlock()
if rc != nil {

36
shared/relay/client/manager_relaystates_test.go
View File

@@ -8,30 +8,15 @@ import (
// TestRelayStates_DoesNotBlockWhileForeignRelayConnecting is a regression test for
// status calls hanging behind an in-progress relay dial.
//
// openConnVia establishes a new foreign relay like this:
//
// rt = NewRelayTrack()
// rt.Lock() // track write-lock
// m.relayClients[serverAddress] = rt
// m.relayClientsMutex.Unlock()
// ...
// err := relayClient.Connect(m.ctx) // network dial, held UNDER rt.Lock()
// ...
// rt.Unlock() // released only after Connect returns/times out
//
// So while a relay is being dialed, its RelayTrack write-lock is held for the whole
// While a relay is being dialed, its RelayTrack write-lock is held for the whole
// dial (up to serverResponseTimeout per transport attempt, times the transport
// fallback chain, times however many relays are being dialed at once).
// fallback chain, times however many relays are being dialed at once) in openConnVia.
//
// RelayStates() — reached from the daemon status path via
// peer.Status.GetFullStatus() -> GetRelayStates() -> Manager.RelayStates() — takes
// rt.RLock() on every tracked relay. A reader lock blocks while a writer holds the
// lock, so a single foreign relay mid-Connect stalls RelayStates(), and therefore
// `netbird status -d`, for the full dial timeout. #6547 moved this off the shared
// map lock but the per-track RLock still blocks the status path.
//
// This test recreates the exact in-progress-dial state (track present in the map
// with its write-lock held) and asserts RelayStates() does not wait on it.
// RelayStates() is reached from the daemon status path via
// peer.Status.GetFullStatus() -> GetRelayStates() -> Manager.RelayStates().
// It takes rt.RLock() on every tracked relay. A reader lock blocks while a
// writer holds the lock, so a single foreign relay mid-Connect in openConnVia
// stalls RelayStates(), and therefore `netbird status -d` hangs for the full dial timeout.
func TestRelayStates_DoesNotBlockWhileForeignRelayConnecting(t *testing.T) {
m := &Manager{
relayClients: make(map[string]*RelayTrack),
@@ -42,8 +27,6 @@ func TestRelayStates_DoesNotBlockWhileForeignRelayConnecting(t *testing.T) {
rt := NewRelayTrack()
rt.Lock()
m.relayClients["relay.example.com:443"] = rt
// Release at the end so a (buggy) blocked RelayStates goroutine can unwind
// instead of leaking past the test.
t.Cleanup(rt.Unlock)
done := make(chan []RelayConnState, 1)
@@ -53,10 +36,7 @@ func TestRelayStates_DoesNotBlockWhileForeignRelayConnecting(t *testing.T) {
select {
case <-done:
// RelayStates returned without waiting for the in-progress dial. Good.
case <-time.After(2 * time.Second):
t.Fatal("RelayStates() blocked on a relay track whose Connect() is in progress " +
"(rt.Lock held across the dial in openConnVia); `netbird status -d` hangs for " +
"the relay dial timeout")
t.Fatal("RelayStates() blocked on a relay track whose Connect() is in progress")
}
}