Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: time: make an uninitialized Timer usable #12721

Closed
cespare opened this issue Sep 22, 2015 · 10 comments

Comments

Projects
None yet
7 participants
@cespare
Copy link
Contributor

commented Sep 22, 2015

Alternatively, add a new function to package time:

// NewStoppedTimer returns a new Timer that never fires unless it is Reset.
func NewStoppedTimer() *Timer

It is common to reuse a timer via Reset several times (usually in a loop); often in these scenarios there's no initial value needed for the timer. It would be nice to be able to do this:

var t time.Timer
for {
    // ...
    t.Reset(x)
    // ...
}

but today any use of a timer that isn't created using NewTimer (or AfterFunc) panics.

Given #11513, almost any "obvious" way of accomplishing this is incorrect:

t := time.NewTimer(0)
for {
    t.Reset(5 * time.Minute)
    <-t.C // might fire immediately, or after 5 minutes
}
t := time.NewTimer(0)
t.Stop()
for {
    t.Reset(5 * time.Minute)
    <-t.C // might fire immediately, or after 5 minutes
}

You need to do something like this instead:

t := time.NewTimer(0)
<-t.C
for {
  t.Reset(5 * time.Minute)
  <-t.C // will not fire immediately
}

but it's unlikely that most people will know that something like this is required, because the bugs in the above versions can be hard to notice.

Compared with t := time.NewTimer(0); <-t.C, either var t time.Timer or time.NewStoppedTimer() are clearer, and the former doesn't even require expanding the time API.

(Note: edited to remove overly-complex workaround.)

@cespare

This comment has been minimized.

Copy link
Contributor Author

commented Sep 22, 2015

Edit: this is obsolete given edits I made to the original post.

@dmac suggested a simpler alternative to my workaround:

t := time.NewTimer(math.MaxInt64)
t.Stop()
@bradfitz

This comment has been minimized.

Copy link
Member

commented Sep 23, 2015

Notice that everything in the time package refers to a Timer as *Timer. It's not used as a value type.

We probably don't want to do anything here.

The caller can use an if statement:

  var t *time.Timer
  for {
       if t == nil {
          t = time.NewTimer(...)
       }
...
@cespare

This comment has been minimized.

Copy link
Contributor Author

commented Sep 23, 2015

@bradfitz sure, t := new(time.Timer) if you prefer. It doesn't change my code examples, as I'm not passing a time.Timer as a function parameter.

What I'm going for is similar to sync.Mutex and sync.WaitGroup.

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Sep 23, 2015

@fjl

This comment has been minimized.

Copy link

commented Sep 24, 2015

Another option is to use

t := time.NewTimer(0)
<-t.C
@cespare

This comment has been minimized.

Copy link
Contributor Author

commented Nov 17, 2015

Thanks @fjl; I've updated my post to remove the complex workaround and took your suggestion.

@funny-falcon

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2017

NewClosedTimer() i think is better.

@bradfitz bradfitz changed the title time: make an uninitialized Timer usable proposal: time: make an uninitialized Timer usable Jan 28, 2017

@bradfitz bradfitz modified the milestones: Proposal, Unplanned Jan 28, 2017

@bradfitz bradfitz added the Proposal label Jan 28, 2017

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2017

There are two kinds of timer: those with an associated func and those with an associated channel. There is no API to change or decide that detail after the timer has been created. That is, a zero Timer would do nothing at all - call no function and send on no channel - when it expires. We could make a zero timer valid for calls to Reset and Stop, but you'd never know when it went off, so what's the point?

If you need a timer that isn't going off any time soon, initialize with time.NewTimer(525600*time.Minute) (or your favorite long amount of time) and then Stop it. Or use nil, as @bradfitz suggested above.

There's no need for new API here.

@cespare

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2017

@rsc good point, I hadn't considered the AfterFunc/NewTimer dichotomy. I was only thinking of channel timers (not func timers).

I think what I'd want is for Stop to do nothing for zero timers and for Reset to associate a channel-sending function (sendTime, in the code) with the timer. In other words, the effect of

t := new(time.Timer)
t.Reset(time.Second)

would be the same as

t := time.NewTimer(time.Second)

It seems to me that preferring "channel timers" over "func timers" in this way is okay since func timers are much less frequently used, and because the documentation already casts func timers as a special case:

When the Timer expires, the current time will be sent on C, unless the Timer was created by AfterFunc.

@cespare

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2017

Since this has been open for a while and doesn't seem to have any support, I'll go ahead and close for now.

@cespare cespare closed this Jan 30, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Aug 15, 2017

Change https://golang.org/cl/14871 mentions this issue: time: allow Reset be called on uninitialized Timer

@golang golang locked and limited conversation to collaborators Aug 15, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.