diff --git a/state/presence/presence.go b/state/presence/presence.go index 398ca4ce1cd..a05dda3ece2 100644 --- a/state/presence/presence.go +++ b/state/presence/presence.go @@ -696,13 +696,6 @@ func (p *Pinger) prepare() error { // sequence in use by the pinger. func (p *Pinger) ping() (err error) { logger.Tracef("pinging %q with seq=%d", p.beingKey, p.beingSeq) - defer func() { - // If the session is killed from underneath us, it panics when we - // try to copy it, so deal with that here. - if v := recover(); v != nil { - err = fmt.Errorf("%v", v) - } - }() session := p.pings.Database.Session.Copy() defer session.Close() if p.delta == 0 { diff --git a/state/presence/presence_test.go b/state/presence/presence_test.go index 70de4f92953..03cc931b916 100644 --- a/state/presence/presence_test.go +++ b/state/presence/presence_test.go @@ -111,6 +111,15 @@ func (s *PresenceSuite) TestErrAndDead(c *gc.C) { } } +// assertStopped stops a worker and waits until it reports stopped. +// Use this method in favor of defer w.Stop() because you _must_ ensure +// that the worker has stopped, and thus is no longer using its mgo +// session before TearDownTest shuts down the connection. +func assertStopped(c *gc.C, w worker.Worker) { + c.Assert(w.Stop(), gc.IsNil) + w.Wait() +} + func (s *PresenceSuite) TestAliveError(c *gc.C) { w := presence.NewWatcher(s.presence, s.modelTag) c.Assert(w.Stop(), gc.IsNil) @@ -118,15 +127,16 @@ func (s *PresenceSuite) TestAliveError(c *gc.C) { alive, err := w.Alive("a") c.Assert(err, gc.ErrorMatches, ".*: watcher is dying") c.Assert(alive, jc.IsFalse) + w.Wait() } func (s *PresenceSuite) TestWorkflow(c *gc.C) { w := presence.NewWatcher(s.presence, s.modelTag) pa := presence.NewPinger(s.presence, s.modelTag, "a") pb := presence.NewPinger(s.presence, s.modelTag, "b") - defer w.Stop() - defer pa.Stop() - defer pb.Stop() + defer assertStopped(c, w) + defer assertStopped(c, pa) + defer assertStopped(c, pb) assertAlive(c, w, "a", false) assertAlive(c, w, "b", false) @@ -194,7 +204,7 @@ func (s *PresenceSuite) TestWorkflow(c *gc.C) { assertChange(c, cha, presence.Change{"a", false}) assertChange(c, chb, presence.Change{"b", false}) - c.Assert(w.Stop(), gc.IsNil) + assertStopped(c, w) } func (s *PresenceSuite) TestScale(c *gc.C) { @@ -220,7 +230,7 @@ func (s *PresenceSuite) TestScale(c *gc.C) { c.Logf("Checking who's still alive...") w := presence.NewWatcher(s.presence, s.modelTag) - defer w.Stop() + defer assertStopped(c, w) w.Sync() ch := make(chan presence.Change) for i := 0; i < N; i++ { @@ -237,8 +247,8 @@ func (s *PresenceSuite) TestScale(c *gc.C) { func (s *PresenceSuite) TestExpiry(c *gc.C) { w := presence.NewWatcher(s.presence, s.modelTag) p := presence.NewPinger(s.presence, s.modelTag, "a") - defer w.Stop() - defer p.Stop() + defer assertStopped(w, c) + defer assertStopped(w, p) ch := make(chan presence.Change) w.Watch("a", ch) @@ -270,8 +280,8 @@ func (s *PresenceSuite) TestWatchPeriod(c *gc.C) { w := presence.NewWatcher(s.presence, s.modelTag) p := presence.NewPinger(s.presence, s.modelTag, "a") - defer w.Stop() - defer p.Stop() + defer assertStopped(c, w) + defer assertStopped(c, p) ch := make(chan presence.Change) w.Watch("a", ch) @@ -315,7 +325,7 @@ func (s *PresenceSuite) TestWatchUnwatchOnQueue(c *gc.C) { func (s *PresenceSuite) TestRestartWithoutGaps(c *gc.C) { p := presence.NewPinger(s.presence, s.modelTag, "a") c.Assert(p.Start(), gc.IsNil) - defer p.Stop() + defer assertStopped(c, p) done := make(chan bool) go func() { @@ -339,7 +349,7 @@ func (s *PresenceSuite) TestRestartWithoutGaps(c *gc.C) { w := presence.NewWatcher(s.presence, s.modelTag) w.Sync() alive, err := w.Alive("a") - c.Check(w.Stop(), gc.IsNil) + assertStopped(c, w) if !c.Check(err, jc.ErrorIsNil) || !c.Check(alive, jc.IsTrue) { break } @@ -369,9 +379,9 @@ func (s *PresenceSuite) TestPingerPeriodAndResilience(c *gc.C) { w := presence.NewWatcher(s.presence, s.modelTag) p1 := presence.NewPinger(s.presence, s.modelTag, "a") p2 := presence.NewPinger(s.presence, s.modelTag, "a") - defer w.Stop() - defer p1.Stop() - defer p2.Stop() + defer assertStopped(c, w) + defer assertStopped(c, p1) + defer assertStopped(c, p2) // Start p1 and let it go on. c.Assert(p1.Start(), gc.IsNil) @@ -399,8 +409,8 @@ func (s *PresenceSuite) TestPingerPeriodAndResilience(c *gc.C) { func (s *PresenceSuite) TestStartSync(c *gc.C) { w := presence.NewWatcher(s.presence, s.modelTag) p := presence.NewPinger(s.presence, s.modelTag, "a") - defer w.Stop() - defer p.Stop() + defer assertStopped(c, w) + defer assertStopped(c, p) ch := make(chan presence.Change) w.Watch("a", ch) @@ -428,8 +438,8 @@ func (s *PresenceSuite) TestStartSync(c *gc.C) { func (s *PresenceSuite) TestSync(c *gc.C) { w := presence.NewWatcher(s.presence, s.modelTag) p := presence.NewPinger(s.presence, s.modelTag, "a") - defer w.Stop() - defer p.Stop() + defer assertStopped(c, w) + defer assertStopped(c, p) ch := make(chan presence.Change) w.Watch("a", ch) @@ -467,8 +477,8 @@ func (s *PresenceSuite) TestSync(c *gc.C) { func (s *PresenceSuite) TestFindAllBeings(c *gc.C) { w := presence.NewWatcher(s.presence, s.modelTag) p := presence.NewPinger(s.presence, s.modelTag, "a") - defer w.Stop() - defer p.Stop() + defer assertStopped(c, w) + defer assertStopped(c, p) ch := make(chan presence.Change) w.Watch("a", ch) @@ -493,12 +503,12 @@ func (s *PresenceSuite) TestFindAllBeings(c *gc.C) { func (s *PresenceSuite) TestTwoEnvironments(c *gc.C) { key := "a" w1, p1, ch1 := s.setup(c, key) - defer w1.Stop() - defer p1.Stop() + defer assertStopped(c, w1) + defer assertStopped(c, p1) w2, p2, ch2 := s.setup(c, key) - defer w2.Stop() - defer p2.Stop() + defer assertStopped(c, w2) + defer assertStopped(c, p2) c.Assert(p1.Start(), gc.IsNil) w1.StartSync()