Timer example using timer.Reset() not working as described

Question

I've been working with examples trying to get my first "go routine" running and while I got it running, it won't work as prescribed by the go documentation with the timer.Reset() function.

In my case I believe that the way I am doing it is just fine because I don't actually care what's in the chan buffer, if anything. All as this is meant to do is trigger case <-tmr.C: if anything happened on case _, ok := <-watcher.Events: and then all goes quiet for at least one second. The reason for this is that case _, ok := <-watcher.Events: can get from one to dozens of events microseconds apart and I only care once they are all done and things have settled down again.

However I'm concerned that doing it the way that the documentation says you "must do" doesn't work. If I knew go better I would say the documentation is flawed because it assumes there is something in the buffer when there may not be but I don't know go well enough to have confidence in making that determination so I'm hoping some experts out there can enlighten me.

Below is the code. I haven't put this up on playground because I would have to do some cleaning up (remove calls to other parts of the program) and I'm not sure how I would make it react to filesystem changes for showing it working.

I've clearly marked in the code which alternative works and which doesn't.

func (pm *PluginManager) LoadAndWatchPlugins() error {

  // DOING OTHER STUFF HERE

    fmt.Println(`m1`)

    done := make(chan interface{})
    terminated := make(chan interface{})

    go pm.watchDir(done, terminated, nil)
    fmt.Println(`m2.pre-10`)

    time.Sleep(10 * time.Second)

    fmt.Println(`m3-post-10`)

    go pm.cancelWatchDir(done)
    fmt.Println(`m4`)

    <-terminated
    fmt.Println(`m5`)

    os.Exit(0) // Temporary for testing

    return Err
}

func (pm *PluginManager) cancelWatchDir(done chan interface{}) {
    fmt.Println(`t1`)

    time.Sleep(5 * time.Second)
    fmt.Println()
    fmt.Println(`t2`)

    close(done)
}

func (pm *PluginManager) watchDir(done <-chan interface{}, terminated chan interface{}, strings <-chan string) {

  watcher, err := fsnotify.NewWatcher()
    if err != nil {
        Logger("watchDir::"+err.Error(), `plugins`, Error)
    }

    //err = watcher.Add(pm.pluginDir)
    err = watcher.Add(`/srv/plugins/`)
    if err != nil {
        Logger("watchDir::"+err.Error(), `plugins`, Error)
    }

    var tmr = time.NewTimer(time.Second)
    tmr.Stop()

    defer close(terminated)
    defer watcher.Close()
    defer tmr.Stop()
    for {
        select {
        case <-tmr.C:
            fmt.Println(`UPDATE FIRED`)
            tmr.Stop()

        case _, ok := <-watcher.Events:
            if !ok {
                return
            }

            fmt.Println(`Ticker: STOP`)
            /*
             *  START OF ALTERNATIVES
             *
             *  THIS IS BY EXAMPLE AND STATED THAT IT "MUST BE" AT:
             *      https://golang.org/pkg/time/#Timer.Reset
             *
             *  BUT DOESN'T WORK
             */
            if !tmr.Stop() {
                fmt.Println(`Ticker: CHAN DRAIN`)
                <-tmr.C // STOPS HERE AND GOES NO FURTHER
            }
            /*
             *  BUT IF I JUST DO THIS IT WORKS
             */
            tmr.Stop()
            /*
             *  END OF ALTERNATIVES
             */

            fmt.Println(`Ticker: RESET`)
            tmr.Reset(time.Second)

        case <-done:
            fmt.Println(`DONE TRIGGERED`)
            return
        }
    }
}

Answer 1

Besides what icza said (q.v.), note that the documentation says:

For example, assuming the program has not received from t.C already:

if !t.Stop() {
        <-t.C
}

This cannot be done concurrent to other receives from the Timer's channel.

One could argue that this is not a great example since it assumes that the timer was running at the time you called t.Stop. But it does go on to mention that this is a bad idea if there's already some existing goroutine that is or may be reading from t.C.

(The Reset documentation repeats all of this, and kind of in the wrong order because Reset sorts before Stop.)

Essentially, the whole area is a bit fraught. There's no good general answer, because there are at least three possible situations during the return from t.Stop back to your call:

• No one is listening to the channel, and no timer-tick is in the channel now. This is often the case if the timer was already stopped before the call to t.Stop. If the timer was already stopped, t.Stop always returns false.
• No one is listening to the channel, and a timer-tick is in the channel now. This is always the case when the timer was running but t.Stop was unable to stop it from firing. In this case, t.Stop returns false. It's also the case when the timer was running but fired before you even called t.Stop, and had therefore stopped on its own, so that t.Stop was not able to stop it and returned false.
• Someone else is listening to the channel.

In the last situation, you should do nothing. In the first situation, you should do nothing. In the second situation, you probably want to receive from the channel so as to clear it out. That's what their example is for.

One could argue that:

if !t.Stop() {
        select {
        case <-t.C:
        default:
        }
}

is a better example. It does one non-blocking attempt that will consume the timer-tick if present, and does nothing if there is no timer-tick. This works whether or not the timer was not actually running when you called t.Stop. Indeed, it even works if t.Stop returns true, though in that case, t.Stop stopped the timer, so the timer never managed to put a timer-tick into the channel. (Thus, if there is a datum in the channel, it must necessarily be left over from a previous failure to clear the channel. If there are no such bugs, the attempt to receive was in turn unnecessary.)

But, if someone else—some other goroutine—is or may be reading the channel, you should not do any of this at all. There is no way to know who (you or them) will get any timer tick that might be in the channel despite the call to Stop.

Meanwhile, if you're not going to use the timer any further, it's relatively harmless just to leave a timer-tick, if there is one, in the channel. It will be garbage-collected when the channel itself is garbage-collected. Of course, whether this is sensible depends on what you are doing with the timer, but in these cases it suffices to just call t.Stop and ignore its return value.

Answer 2

You create a timer and you stop it immediately:

var tmr = time.NewTimer(time.Second)
tmr.Stop()

This doesn't make any sense, I assume this is just an "accident" from your part.

But going further, inside your loop:

    case _, ok := <-watcher.Events:

When this happens, you claim this doesn't work:

        if !tmr.Stop() {
            fmt.Println(`Ticker: CHAN DRAIN`)
            <-tmr.C // STOPS HERE AND GOES NO FURTHER
        }

Timer.Stop() documents that it returns true if this call stops the timer, and false if the timer has already been stopped (or expired). But your timer was already stopped, right after its creation, so tmr.Stop() returns false properly, so you go inside the if and try to receive from tmr.C, but since the timer was "long" stopped, nothing will be sent on its channel, so this is a blocking (forever) operation.

If you're the one stopping the timer explicitly with timer.Stop(), the recommended "pattern" to drain its channel doesn't make any sense and doesn't work for the 2nd Timer.Stop() call.