Fix RingBuffer shutdown #620
Conversation
Fixes RingBuffer's Stop to do a clean exit. Adds a polling 300ms timeout to check for stop condition. The Stop() method waits for the polling goroutine to exit before returning to the caller.
eyakubovich
force-pushed
the
fix-ringbuf-stop
branch
from
07deed6
to
09b2b47
Compare
There was a problem hiding this comment.
Thanks so much for this @eyakubovich! Figuring out how to avoid blocking while completely avoiding a race condition at shutdown was throwing me for quite a loop. It's a great feeling not only to get a code contribution but to also learn from it.
Can you share the command(s) you used for running the race detector against this? I'd love to add it to our github actions CI/CD.
| // may have stopped at this point. Failure to drain it will | ||
| // result in a deadlock: the channel will fill up and the poll | ||
| // goroutine will block in the callback. | ||
| eventChan := eventChannels[uintptr(rb.bpfMap.fd)] |
There was a problem hiding this comment.
Ah this was the pattern that I failed to recognize the need for in previous iterations of this code. I couldn't get past the fact that the callback was blocking (and the timeout passed to ring_buffer__poll would not interrupt it).
|
The race detector ran as part of the unit tests ( There's actually another race: |
|
Got it! If you'd like to work on that it's more than welcome, looking into it right now just to understand it. Is that project open source? If so could you share it? |
|
Yes, it's open source and called Teleport (https://github.com/gravitational/teleport). |
|
@eyakubovich Thanks for the info. Please feel free to file issues as you work on moving it over. I'm actively adding functionality to libbpf, among other things I'm hoping to have the full libbpf API implemented in the next couple months. |
Fixes RingBuffer's Stop to do a clean exit.
Adds a polling 300ms timeout to check for stop condition.
The Stop() method waits for the polling goroutine to exit
before returning to the caller.
Fixes #619