~niemeyer/juju-core/ignore-me

Viewing all changes in revision 219.

  • Committer: Gustavo Niemeyer
  • Author(s): William Reade
  • Date: 2012-06-13 14:55:34 UTC
  • mfrom: (214.3.7 juju)
  • Revision ID: gustavo@niemeyer.net-20120613145534-0f42rq7ztrax1elo
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

expand all expand all

Show diffs side-by-side

added added

removed removed

Lines of Context: