Skip to content

Commit

Permalink
Fix potential p2p shutdown hangup (#10626)
Browse files Browse the repository at this point in the history
This is a fix for: 

#10192

This fixes is a deadlock in v4_udp.go where 
* Thread A waits on mutex.Lock() in resetTimeout() called after reading
listUpdate channel.
* Thread B waits on listUpdate <- plist.PushBack(p) called after locking
mutex.Lock()
  
This fix decouples the list operations which need locking from the
channel operations which don't by storing the changes in local
variables. These updates are used for resetting a timeout - which is not
order dependent.
  • Loading branch information
mh0lt authored and yperbasis committed Jun 21, 2024
1 parent 269bd3d commit 5a58bda
Showing 1 changed file with 12 additions and 7 deletions.
19 changes: 12 additions & 7 deletions p2p/discover/v4_udp.go
Original file line number Diff line number Diff line change
Expand Up @@ -606,14 +606,15 @@ func (t *UDPv4) loop() {
return

case p := <-t.addReplyMatcher:
func() {
mutex.Lock()
defer mutex.Unlock()
p.deadline = time.Now().Add(t.replyTimeout)
listUpdate <- plist.PushBack(p)
}()
mutex.Lock()
p.deadline = time.Now().Add(t.replyTimeout)
back := plist.PushBack(p)
mutex.Unlock()
listUpdate <- back

case r := <-t.gotreply:
var removals []*list.Element

func() {
mutex.Lock()
defer mutex.Unlock()
Expand All @@ -629,7 +630,7 @@ func (t *UDPv4) loop() {
if requestDone {
p.errc <- nil
plist.Remove(el)
listUpdate <- el
removals = append(removals, el)
}
// Reset the continuous timeout counter (time drift detection)
contTimeouts = 0
Expand All @@ -638,6 +639,10 @@ func (t *UDPv4) loop() {
r.matched <- matched
}()

for _, el := range removals {
listUpdate <- el
}

case key := <-t.gotkey:
go func() {
if key, err := v4wire.DecodePubkey(crypto.S256(), key); err == nil {
Expand Down

0 comments on commit 5a58bda

Please sign in to comment.