state/watcher: fix bug in UnwatchCollection

state/watcher: fix bug in UnwatchCollection

An outstanding request being sent to a collection
watch would not be cancelled by UnwatchCollection.

https://code.launchpad.net/~rogpeppe/juju-core/240-state-watcher-unwatchcollection-fix/+merge/152432

(do not edit description out of merge proposal)

[rog, 2013-03-08 15:31:29]

Please take a look.

[dimitern, 2013-03-08 15:40:26]

Good catch! LGTM.

One question: could this possibly fix this bug as well?

https://bugs.launchpad.net/juju-core/+bug/1135444
<state/watcher intermittent test failure>

[dave_cheney.net, 2013-03-08 20:26:59]

LGTM. Nice catch.

https://codereview.appspot.com/7610044/diff/2001/state/watcher/watcher.go
File state/watcher/watcher.go (right):

https://codereview.appspot.com/7610044/diff/2001/state/watcher/watcher.go#new...
state/watcher/watcher.go:78: // to a particular item.
nit: s/match/matches

[jameinel, 2013-03-08 20:56:17]

I like this, but I did test running the test without the fix, and it still
passed. It would be good to have a test that explicitly affects this, so that it
doesn't regress in the future.
Is it timing related?

[rog, 2013-03-08 21:33:21]

Please take a look.

[rog, 2013-03-08 21:51:31]

https://codereview.appspot.com/7610044/diff/2001/state/watcher/watcher.go
File state/watcher/watcher.go (right):

https://codereview.appspot.com/7610044/diff/2001/state/watcher/watcher.go#new...
state/watcher/watcher.go:78: // to a particular item.
On 2013/03/08 20:26:59, dfc wrote:
> nit: s/match/matches

i started with "matches" but changed it because i think "match" is probably more
idiomatic (c.f. http://golang.org/pkg/regexp/#Regexp.Match)

i'll change it if you still think it would be better as "matches" though.

[fwereade, 2013-03-09 15:24:35]

LGTM. I kinda favour "matches" but it's up to you.

[rog, 2013-03-15 15:04:00]

*** Submitted:

state/watcher: fix bug in UnwatchCollection

An outstanding request being sent to a collection
watch would not be cancelled by UnwatchCollection.

R=dimitern, dfc, jameinel, fwereade
CC=
https://codereview.appspot.com/7610044