Fix data race in WithContext with cancelled parent #20
Conversation
|
EDIT (after @niemeyer's gentle rebuke): I apologize for the above comment. I have no excuse. |
|
Sorry for not responding timely. Project is used in several large and critical production systems. Being responsible for those systems is what keeps maintainers busy elsewhere. |
|
I apologize if I came off snarky. That wasn't my intent. I hadn't seen any traction on any PR's for quite some time and assumed that this project was no longer actively maintained. My comment wasn't intended to be a critique of the maintainers. |
|
I am more than happy to reopen this. There is at least one other race condition that I found that I would be happy to address in a future PR if there is interest. |
|
Okay, let's reopen it then, thanks. Will have a look next week once I'm back from the current event. Please ping if I miss it. Certainly happy to take the other PR as well, if there is a known race. Thanks for that too. |
32a061e to
5909db5
Compare
|
I just remembered I had this open :) Took a fresh look after these few years and verified that the data race still existed. I think my new solution in the latest commit is more correct. |
|
I've also updated the PR description and title to match the new solution. |
If the parent context is already cancelled, or cancelled before WithContext returns, there is a data race that can happen between the goroutine that is spawned to detect when the parent context is cancelled or the tomb is dying, which calls Kill() and the code in WithContext that updates the children. This change fixes the race by not kicking off the goroutine that waits for parent cancellation until after the rest of the initialization has been completed.
5909db5 to
3850328
Compare
If the parent context is already cancelled, or cancelled before WithContext returns, there is a data race that can happen between the goroutine that is spawned to detect when the parent context is cancelled or the tomb is dying, which calls Kill() and the code in WithContext that updates the children.
This change fixes the race by not kicking off the goroutine that waits for parent cancellation until after the rest of the initialization has been completed.