How to stop a timer correctly?

Question

var timer *time.Timer

func A() {
    timer.Stop() // cancel old timer
    go B() // new timer
}

func B() {
    timer = time.NewTimer(100 * time.Millisecond)
    select {
    case <- timer.C:
    // do something for timeout, like change state
    }
}

Function A and B are all in different goroutines.

Say A is in a RPC goroutine. When application receives RPC request, it will cancel the old timer in B, and start a new timer in another goroutine.

The doc say:

Stop does not close the channel, to prevent a read from the channel succeeding incorrectly.

So how to break the select in B to avoid goroutine leak?

Solution 1

Use an additional, independent cancellation signal. Since you already have a select statement in place, another channel is an obvious choice:

import "time"

var timer *time.Timer
var canceled = make(chan struct{})

func A() {
    // cancel all current Bs
    select {
    case canceled <- struct{}{}:
    default:
    }   

    timer.Stop()

    go B()       // new timer
}

func B() {
    timer = time.NewTimer(100 * time.Millisecond)
    select {
    case <-timer.C:
        // do something for timeout, like change state
    case <-canceled:
        // timer aborted
    }
}

Note that all As and Bs race against each other for the timer value. With the code above it is not necessary to have A stop the timer, so you don't need a global timer, eliminating the race:

import "time"

var canceled = make(chan struct{})

func A() {
    // cancel all current Bs
    select {
    case canceled <- struct{}{}:
    default:
    }

    go B()
}

func B() {
    select {
    case <-time.After(100 * time.Millisecond):
        // do something for timeout, like change state
    case <-canceled:
        // aborted
    }
}

Solution 2

Adding to the above answer, if you want to cancel all waiters at once, you can encapsulate the behavior using your own timer mechanism that can be cancelled, that sends true or false in an After channel to tell you whether you are waking from a cancellation or a time out, for all waiters.

package main

import (
    "fmt"
    "time"
)

type CancellableTimer struct {
    cancel chan bool
}

func NewCancellableTimer() *CancellableTimer {
    return &CancellableTimer{
        cancel: make(chan bool),
    }
}

// internal wait goroutine wrapping time.After
func (c *CancellableTimer) wait(d time.Duration, ch chan bool) {
    select {
    case <-time.After(d):
        ch <- true
    case <-c.cancel:
        ch <- false
    }
}

// After mimics time.After but returns bool to signify whether we timed out or cancelled
func (c *CancellableTimer) After(d time.Duration) chan bool {
    ch := make(chan bool)
    go c.wait(d, ch)
    return ch
}

// Cancel makes all the waiters receive false
func (c *CancellableTimer) Cancel() {
    close(c.cancel)

}

// a goroutine waiting for cancellation
func B(t *CancellableTimer) {
    select {
    // timedOut will signify a timeout or cancellation
    case timedOut := <-t.After(time.Second):
        if timedOut {
            fmt.Println("Time out!")
        } else {
            fmt.Println("Cancelled!")
        }
    }
}

func main() {
    t := NewCancellableTimer()
    // Start 3 goroutines that wait for different timeouts on the same timer
    go B(t)
    go B(t)
    go B(t)

    // sleep a bit before cancelling
    time.Sleep(100 * time.Millisecond)

    // cancel the timer and all its waiters
    t.Cancel()

    // this is just to collect the output
    time.Sleep(time.Second)

}

Output:

Cancelled!
Cancelled!
Cancelled!

playground link:

https://play.golang.org/p/z8OscJCXTvD

Comments

[Dima Tisnek]

Please describe the high level problem you are trying to solve.

[Shahriar]

Can you elaborate your question?

[lxyscls]

@aerokite When timeout, switch to next state. when receive request, keep the state and start a new timer.

[zerkms]

@lxyscls what real problem do you have with this code though? The 100ms timer is not stopped. So what?

[Not_a_Golfer]

Just use a cancellation channel and select on both

[lxyscls]

@zerkms When timeout, application will change state, just like in some consensus protocol. Say follower to candidate.

[zerkms]

@lxyscls you need to have 2 branches in select. Select with 1 branch makes no sense. As soon as you add something in the other - your question would not be actual anymore.

[lxyscls]

I'am doubt that "canceled <- struct{}{}" can "cancel all current Bs”. Not a random B?

[Not_a_Golfer]

@lxyscls if you close the cancel channel instead of pushing to it, every goroutine will exit.

[Peter]

@lxyscls, you are right. Not sure what I was thinking. However closing the channel will also abort all future B's.