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

context: document WithCancel asynchronous cancellation #33185

Closed
egonelbre opened this issue Jul 19, 2019 · 10 comments
Closed

context: document WithCancel asynchronous cancellation #33185

egonelbre opened this issue Jul 19, 2019 · 10 comments
Labels
Documentation FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@egonelbre
Copy link
Contributor

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 version
go version go1.13beta1 windows/amd64
go env Output
$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=Z:\gocache
set GOENV=C:\Users\egone\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=F:\Go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=Z:\Temp\go-build959337302=/tmp/go-build -gno-record-gcc-switches
@ianlancetaylor
Copy link
Contributor

I think this is basically a variant of #28728. The problem can only arise with a user-defined implementation of Context.

@OneOfOne
Copy link
Contributor

@ianlancetaylor but is it by design to prevent user-defined impl?

@ianlancetaylor
Copy link
Contributor

Not to my knowledge. I don't see anything in #28728 that suggests that.

@katiehockman
Copy link
Contributor

@OneOfOne are you good with tracking this issue in #28728 then? If so, I can close this issue and keep the conversation going there.

@egonelbre
Copy link
Contributor Author

@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.

@katiehockman katiehockman added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jul 29, 2019
@katiehockman
Copy link
Contributor

@egonelbre thanks for clarifying!

@OneOfOne
Copy link
Contributor

@katiehockman yep I'm fine with that.

@gopherbot
Copy link

Change https://golang.org/cl/187921 mentions this issue: context: mention asynchronous cancellation propagation

@ianlancetaylor
Copy link
Contributor

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?

@egonelbre
Copy link
Contributor Author

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.

@golang golang locked and limited conversation to collaborators Sep 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

5 participants