-
Notifications
You must be signed in to change notification settings - Fork 18k
context: document WithCancel asynchronous cancellation #33185
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Comments
I think this is basically a variant of #28728. The problem can only arise with a user-defined implementation of |
@ianlancetaylor but is it by design to prevent user-defined impl? |
Not to my knowledge. I don't see anything in #28728 that suggests that. |
@katiehockman this issue is just about the documentation. The proposal for changing the API seems somewhat orthogonal since as far as I understand it doesn't eliminate the asynchronous cancellation. |
@egonelbre thanks for clarifying! |
@katiehockman yep I'm fine with that. |
Change https://golang.org/cl/187921 mentions this issue: |
Is this really worth documenting in the package docs? It seems more confusing than helpful. Would those docs really have helped avoid the bug hunt you mentioned? |
Might have cut down a little time for the bug hunt. If it were mentioned in context documentation from the start, I might have known it beforehand. But, this is just speculation. I understand that it's a corner case behavior and understand the need to keep documentation clear as possible... so, I can drop the PR and accept the "wontfix" resolution, if people think it makes things more confusing. |
Using context.WithCancel cancellation may happen asynchronously however it is not documented.
A facilitated example of this happening: https://play.golang.org/p/ji63DpfJmMg (running locally it sometimes finishes in time). The corresponding code triggering this behavior is in https://github.com/golang/go/blob/master/src/context/context.go#L262.
I was cancelling a context and expected all child-contexts to be cancelled immediately. This caused a "bug hunt". I'm not sure whether there is a way to fix this, so it's probably worth at least documenting this asynchronous behavior.
Details
go env
OutputThe text was updated successfully, but these errors were encountered: