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

Jump to bottom

proposal: Add time.AfterContext #40736

Closed
nhooyr opened this issue Aug 12, 2020 · 5 comments
Closed

proposal: Add time.AfterContext #40736

nhooyr opened this issue Aug 12, 2020 · 5 comments
Labels
FrozenDueToAge Proposal
Milestone
Proposal

Comments

@nhooyr
Copy link
Contributor

nhooyr commented Aug 12, 2020

Often times I've found myself wanting to have a delay in my program that is interruptible.

Presently I have to write such code as:

t := time.NewTimer(defaultStatInterval)
defer t.Stop()

select {
case <-t.C:
case <-ctx.Done():
  return false, ctx.Err()
}

This is verbose and easy to get wrong if you use time.After instead of time.NewTimer in an attempt to make it shorter as you'll end up leaking timers when the context is cancelled.

I propose we add the following function to the time package to make the above use case easier and safer.

// AfterContext is just like After but can be interrupted by the passed ctx.
// If the context is cancelled, the channel will be closed.
func AfterContext(ctx context.Context, dur time.Duration) <-chan Time

I couldn't come up with a good use case or API for AfterFunc with context interruption so I left it out.

Example

<-time.AfterContext(ctx, time.Second*5)
if ctx.Err() != nil {
    return
}

// Do stuff with ctx here.

Caveat

The name is certainly confusing. Please comment if you have a better suggestion.
@gopherbot gopherbot added this to the Proposal milestone Aug 12, 2020
@seankhliao
Copy link
Member

Have you considered:

ctx, cancel := context.WithTimeout(ctx, time.Second*5)
defer cancel()
<-ctx.Done()
if ctx.Err() != nil {
    return
}

@nhooyr
Copy link
Contributor Author

nhooyr commented Aug 12, 2020

@seankhliao

While that's certainly shorter, in your example you're shadowing the parent ctx which you'll want to use afterwards. You could use two different contexts but it's easy to get confused between them. I think the proposed function fits nicely in the time package and is much clearer/easier to use.

It'd help this proposal a lot if I had some objective data on how often this comes up in practice but I don't and I'm not really sure how I would go about doing so.

@seankhliao
Copy link
Member

Note the proposed API would be impossible due to the import cycle between time and context

Also don't think this fits into the time package as it doesn't deal strictly with time

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Aug 12, 2020
@ianlancetaylor
Copy link
Contributor

It seems to me that this can be written outside of the standard library. https://golang.org/doc/faq#x_in_std

@nhooyr
Copy link
Contributor Author

nhooyr commented Aug 13, 2020 •
edited

Note the proposed API would be impossible due to the import cycle between time and context

Very true.

Also don't think this fits into the time package as it doesn't deal strictly with time

Not sure what you mean by this, it is dealing directly with time?

Anyhow, will close as like you mentioned, the proposed API isn't going to work. We could get around the import cycle with some bank but I don't think that's justified for a single function.

@nhooyr nhooyr closed this as completed Aug 13, 2020
@martisch martisch removed this from Incoming in Proposals (old) Aug 14, 2020
@golang golang locked and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
4 participants
@ianlancetaylor @gopherbot @nhooyr @seankhliao