DescriptionMake 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).
https://code.launchpad.net/~fwereade/juju-core/rework-stop-watchers/+merge/109877
(do not edit description out of merge proposal)
Patch Set 1 #Patch Set 2 : Make watcher termination clearer, safer #Patch Set 3 : Make watcher termination clearer, safer #Patch Set 4 : Make watcher termination clearer, safer #Patch Set 5 : Make watcher termination clearer, safer #
MessagesTotal messages: 3
|