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: context: add Context.IsCanceled method #39893

Open
ilyapashuk opened this issue Jun 27, 2020 · 6 comments
Open

proposal: context: add Context.IsCanceled method #39893

ilyapashuk opened this issue Jun 27, 2020 · 6 comments
Labels
Projects
Milestone

Comments

@ilyapashuk
Copy link

@ilyapashuk ilyapashuk commented Jun 27, 2020

please add function like that to the context type:
func (c Context) IsCanceled() {
select {
case <- c.Done():
return true
default:
return false
}
}
or something like that to simplify context cancelation checking in channelless situations by something like that:
if ctx.IsCanceled() {
return ctx.Err()
}

@gopherbot gopherbot added this to the Proposal milestone Jun 27, 2020
@gopherbot gopherbot added the Proposal label Jun 27, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 27, 2020

The proposed function is the same as ctx.Err() != nil. It doesn't seem to add much. In particular the case at the bottom would normally be written

    if err := ctx.Err(); err != nil {
        return err
    }

which seems preferable to me.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Jun 27, 2020
@ianlancetaylor ianlancetaylor changed the title proposal: add IsCanceled function to context type proposal: context: add Context.IsCanceled method Jun 27, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Jul 8, 2020

As Ian pointed out, ctx.Err is exactly this function (returning an error instead of a boolean).
In #19856, we made sure that this idiom is valid - Err is allowed before the Done channel is closed, and it must return nil.

@rsc rsc moved this from Incoming to Active in Proposals Jul 8, 2020
@carnott-snap
Copy link

@carnott-snap carnott-snap commented Jul 8, 2020

While you could implement a func that is pure syntax sugar, I agree the justification seems weak:

package context

func IsCanceled(ctx Context) bool { return ctx.Err() == Canceled }
@rsc
Copy link
Contributor

@rsc rsc commented Jul 15, 2020

Based on the discussion above, this seems like a likely decline.

@rsc rsc moved this from Active to Likely Decline in Proposals Jul 15, 2020
@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 16, 2020

There’s also the issue of a logical race, the context may be canceled after the call to IsCancelled but before the code responding to that condition has run.

@rsc
Copy link
Contributor

@rsc rsc commented Jul 22, 2020

No change in consensus, so declined.

@rsc rsc moved this from Likely Decline to Declined in Proposals Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Declined
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.