Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data Race in UDP Frontend #608

Open
mrd0ll4r opened this issue Feb 8, 2023 · 0 comments · May be fixed by #618
Open

Data Race in UDP Frontend #608

mrd0ll4r opened this issue Feb 8, 2023 · 0 comments · May be fixed by #618

Comments

@mrd0ll4r
Copy link
Member

mrd0ll4r commented Feb 8, 2023

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
golang/go#23842

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant