Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix race found by race detector, and reported by Dave Cheney.
Here is more information than you'll want to know: If goroutine 1 is doing: t.Kill(nil) t.Wait() and goroutine 2 is doing: t.Wait() and goroutine 3 is doing: t.Done() There is a race, because goroutine 3 could finish first, and then goroutine 1 might run, and the memory model won't guarantee that goroutine 2 actually sees t.reason recorded by the Kill(nil) of goroutine 1. This specific case is more of a theoretical problem than a real one, for the following reasons: - The memory model guarantees that the close(t.done) performed by t.Done() will Happen Before the <-t.done done by either t.Wait, which means both goroutine 1 and 2 will always necessarily see a real error coming out of the goroutine(s) being monitored by t. - The Kill(nil) performed by goroutine 1 won't affect the value of t.reason due to the logic in the Kill method (although, observing the memory model pedantically means goroutine 1 might do whatever it pleases with the memory region, due to lack of synchronization). In a different scenario, though, t.Kill in goroutine 1 might be performed with a non-nil error, and goroutine 2 would see either nil or the error, and t.reason might be in an intermediate unknown state, so the race is in fact real. Solving this specific race via a mutex, as done in this change, still won't mean that this behavior is sane, though: what t.Wait() in goroutine 2 observes will be either the prior error value, or the new error value, based purely on timing of the two Wait calls. The change introduced protects t.reason with the tomb's mutex, which silents the race detector, and makes the logic entirely memory-model friendly.
- Loading branch information