From 0167b4b4a3bf120224a7c71e4c387fb058d939b8 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 17 Nov 2015 12:03:18 +0100 Subject: [PATCH] Don't cause rare spurious event timeout Correctly resetting timers is surprisingly tricky. --- lib/events/events.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/events/events.go b/lib/events/events.go index 29391fec..5ad94e33 100644 --- a/lib/events/events.go +++ b/lib/events/events.go @@ -164,11 +164,20 @@ func (l *Logger) Log(t EventType, data interface{}) { func (l *Logger) Subscribe(mask EventType) *Subscription { l.mutex.Lock() dl.Debugln("subscribe", mask) + s := &Subscription{ mask: mask, events: make(chan Event, BufferSize), timeout: time.NewTimer(0), } + + // We need to create the timeout timer in the stopped, non-fired state so + // that Subscription.Poll() can safely reset it and select on the timeout + // channel. This ensures the timer is stopped and the channel drained. + if !s.timeout.Stop() { + <-s.timeout.C + } + l.subs = append(l.subs, s) l.mutex.Unlock() return s @@ -196,19 +205,18 @@ func (l *Logger) Unsubscribe(s *Subscription) { func (s *Subscription) Poll(timeout time.Duration) (Event, error) { dl.Debugln("poll", timeout) - if !s.timeout.Reset(timeout) { - select { - case <-s.timeout.C: - default: - } - } + s.timeout.Reset(timeout) select { case e, ok := <-s.events: if !ok { return e, ErrClosed } - s.timeout.Stop() + if !s.timeout.Stop() { + // The timeout must be stopped and possibly drained to be ready + // for reuse in the next call. + <-s.timeout.C + } return e, nil case <-s.timeout.C: return Event{}, ErrTimeout