chihaya / chihaya

A customizable, multi-protocol BitTorrent Tracker
https://chihaya.io
Other
1.43k stars 190 forks source link

Data Race in UDP Frontend #608

Open mrd0ll4r opened 1 year ago

mrd0ll4r commented 1 year ago

The data race in the UDP Frontend seems to be because any combination of these:

  1. we call (*sync.WaitGroup).Add from a different goroutine than the one calling (*sync.WaitGroup).Wait, e.g., here and here
  2. we call (*sync.WaitGroup).Add after a call to (*sync.WaitGroup).Wait has started, i.e., here. This seems more likely to me. We set the read deadline on the socket and call Wait, but serve() still has some packet data and launches a goroutine to process that.

I also noticed that (t *Frontend).Stop() is not thread-safe by my definition, because it could close t.closing multiple times, which would panic.

In general, that needs a small rework with

  1. A mutex to govern write-access to closing
  2. A signalling/synchronization mechanism that ensures we will never call (*sync.WaitGroup).Add after we set the socket read deadline (or some other useful point)
  3. Probably moving this outside of the goroutine, because it really is a bit weird to have it in there.

EDIT relevant links: https://pkg.go.dev/sync#WaitGroup.Add https://github.com/golang/go/issues/23842