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

x/net/http2: clarifying concurrent Close on requestBody #17589

Closed
voutasaurus opened this issue Oct 25, 2016 · 3 comments
Closed

x/net/http2: clarifying concurrent Close on requestBody #17589

voutasaurus opened this issue Oct 25, 2016 · 3 comments
Milestone

Comments

@voutasaurus
Copy link

@voutasaurus voutasaurus commented Oct 25, 2016

What version of Go are you using (go version)?

go version go1.7.1 darwin/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/voutasaurus"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/v_/gz6l428x5w97h0zrm5m03rlm0000gn/T/go-build981066443=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

Read golang/net@40a0a18

What did you expect to see?

Based on the commit message, support for concurrent calls to requestBody.Close.

What did you see instead?

A closed bool field that makes Close() idempotent but not goroutine safe.

Supporting concurrent Close() may not be worth the effort. I've been bitten by it once due to defers and timeouts but that was my fault and now context makes that stuff much simpler. The commit message could be clearer about what the closed bool is for though.

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Oct 25, 2016

io.Closer and io.Reader are not in general safe for concurrent uses.

Does this affect any code in the wild?

@rakyll rakyll added this to the Go1.8 milestone Oct 25, 2016
@rakyll rakyll changed the title golang/net/http2: clarifying concurrent Close on requestBody x/net/http2: clarifying concurrent Close on requestBody Oct 25, 2016
@voutasaurus

This comment has been minimized.

Copy link
Author

@voutasaurus voutasaurus commented Oct 25, 2016

Does this affect any code in the wild?

Not to my knowledge

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Oct 25, 2016

Okay, then I'm going to close this.

@bradfitz bradfitz closed this Oct 25, 2016
@golang golang locked and limited conversation to collaborators Oct 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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