223.1.1
|
|
|
Roger Peppe |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
223
|
|
|
William Reade |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
222
|
|
|
William Reade |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
221
|
|
|
Roger Peppe |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
220
|
|
|
Frank Mueller |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
219
|
|
state: make watcher termination clearer, safer
The existing watcher implementations shared a flawed implementation style: specifically, that closure of the input channel was not treated as an error. This is presumably because the Stop method also stopped the watcher that provided the input channel, and this could happen at any time whatsoever (from the perspective of the loop goroutine); however, this then meant that the loop goroutine treated any of the following conditions:
* watcher itself was stopped by tomb.Kill() * subwatcher was stopped by tomb.Kill() * subwatcher channel closed for some other reason
...as a sign of clean shutdown; that is to say that all subwatcher errors were being swallowed silently.
This CL avoids that problem by deferring all cleanup work, including stoppage of the subwatcher, to the end of the loop goroutine; and, in the loop goroutine, treating <-w.tomb.Dying() as the only reason to terminate cleanly. Unexpected closures of the input channel are hence correctly detected as errors; and expected closures (caused by cleanup) will never be detected, because we will always have exited the goroutine loop by the time we stop the subwatcher.
As a consequence, there is no need to wait on subwatcher death: if it dies while the loop is running the error will be detected soon enough via the closure of the input channel; but that's not really any reason to abort a send on the output channel.
Original discussion on https://codereview.appspot.com/6300085/ (which I appear to have proposed for the wrong target somehow).
R= CC= https://codereview.appspot.com/6307081
|
William Reade |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
218
|
|
|
Dave Cheney |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
217
|
|
|
Roger Peppe |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
216
|
|
|
Roger Peppe |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
215
|
|
|
Roger Peppe |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
214
|
|
|
Dave Cheney |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
213
|
|
|
Roger Peppe |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
212
|
|
|
Gustavo Niemeyer |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
211
|
|
|
Gustavo Niemeyer |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
210
|
|
|
Frank Mueller |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
209
|
|
|
Gustavo Niemeyer |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
208
|
|
|
William Reade |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
207
|
|
|
Frank Mueller |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
206
|
|
|
William Reade |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|
205
|
|
|
William Reade |
12 years ago
|
![Diff](/static/images/ico_diff.gif) |
|