timer Reset concurrent to receive

[Tim Hockin]

I'm not convinced that the docs quite cover the case I am looking, so
I am posting here.

https://golang.org/pkg/time/#Timer.Reset says "This should not be done
concurrent to other receives from the Timer's channel" but it's not
clear what the repercussions are.

In our case, I have a function to be run periodically, on a timer, but
it can be run manually too. When run manually, I want to push the
timer out (restart the period).

I have a goroutine doing:

```
for {
select {
case <-stop:
pr.stop()
return
case <-timer.C:
run()
}
}
```

deep inside run(), we have:

```
timer.Stop()
timer.Reset(period)
```

I understand that I could lose the race and deliver on timer.C _just
before_ this runs, and that is fine. What I am seeking to know is
whether this is considered "safe"? The receive is running
concurrently to the Reset(). Will this cause problems inside Timer,
beyond the potential "extra" delivery? Do I need to break the loop
and stop receiving on it while the Reset() happens?

Tim

[Ian Lance Taylor]

The sentence "This should not be done concurrent to other receives
from the Timer's channel." is intended to apply to the description of
how to use t.Stop safely. It's there because if you use the code
fragment described there and there is a concurrent receive, you don't
know which channel receive will succeed. In other words, the channel
receive in the code fragment might hang.

It is safe to use Reset as you describe, as long as you understand
that the timer may expire, and send a value to the channel, as you
call Stop and Reset. If you don't care about that--if an extra value
sent to the channel doesn't matter--then your code is fine.

Ian

[Tim Hockin]

Thanks! That makes sense. Does it make sense to update the docs to
show the "select-with-default" mode of draining the channel instead?

[Ian Lance Taylor]

On Wed, Jan 25, 2017 at 10:30 AM, Tim Hockin <tho...@google.com> wrote:
> Thanks! That makes sense. Does it make sense to update the docs to
> show the "select-with-default" mode of draining the channel instead?

I guess I don't think so, as there is still a potential race with the
other receive. I mean, we can make the docs arbitrarily complicated,
but at some point it should be on the wiki or something.

Ian

[Tim Hockin]

Better to have a documented race than a potential hang, though, yeah?
If it delivers and then I Stop(), drain, Reset(), I might lose the
race, have the channel read by the concurrent receiver, and and just
block here. Or am I missing some other nuance here?

[Ian Lance Taylor]

On Wed, Jan 25, 2017 at 11:32 AM, Tim Hockin <tho...@google.com> wrote:
> Better to have a documented race than a potential hang, though, yeah?
> If it delivers and then I Stop(), drain, Reset(), I might lose the
> race, have the channel read by the concurrent receiver, and and just
> block here. Or am I missing some other nuance here?

The docs right now say: don't do this with a concurrent receiver. You
are talking about the case where there is a concurrent receiver. I
agree that that case is more complicated and requires further details,
I'm just not sure the docs for this package are the place to discuss
them.

[Tim Hockin]

I'm not going to debate it since I am not really volunteering to fix
the docs right now, anyway :) My sense is that when I want to know
how to use something, I run `go doc` on it. The doc as it stands says
"don't do it" when the reality has an extra "unless ..." clause.

Thanks for the guidance!