trace: avoid race between event recycling and freeTrace() #75
Conversation
…Trace() When there are less than 4 events in a trace, tr.events is backed by the tr.eventsBuf array. This leads to a race between the background recycler invocation in unref(). Sometimes tr.events just consists of empty event objects when the recycler finally gets to them. So in the case where the array is shared, we make a copy of the events slice, so that recycling can be done in the background but freeTrace can be run immediately.
|
This PR (HEAD: 5e073f0) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/net/+/238302 to see it. Tip: You can toggle comments from me using the |
|
Message from Gobot Gobot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/238302. |
|
Message from Go Bot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/238302. |
When there are less than 5 events in a trace, the
tr.eventsslice is backed by thetr.eventsBufarray. This leads to a race between the background recycler invocation inunref(). IffreeTrace()runs before the recycler goroutine,tr.eventsjust consists of empty event objects when the recycler finally gets to them, and thus the user's recycling function cannot do any useful recycling (asevent.Whatis now nil). So in the case where the array is shared, we make a copy and pass that to the recycler, so that recycling can be done in the background butfreeTracecan be run immediately.Since the copying is only done when there are less than 5 events to copy, it should not have a significant performance impact.
I added a bunch of test cases around this, perhaps too many. And, you will be horrified at how we wait for all the recyclers to run in the test. I did this to avoid modifying the code to add synchronization between the entire recycling operation and the test, and to avoid the test hanging for a long time when it's in the failing state. Suggestions for improvement would be most welcome!