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

net/http: data race reading a "Expect: 100-Continue" request while writing response #13050

Closed
linkdata opened this issue Oct 26, 2015 · 6 comments

Comments

Projects
None yet
5 participants
@linkdata
Copy link

commented Oct 26, 2015

The goroutine reading the request body will be updating "ecr.sawEOF", while the goroutine writing the response will be reading that boolean with no synchronization.

Using go1.5.1 windows/amd64.

WARNING: DATA RACE
Read by goroutine 438:
  net/http.(*chunkWriter).writeHeader()
      C:/workdir/go/src/net/http/server.go:866 +0xe38
  net/http.(*chunkWriter).Write()
      C:/workdir/go/src/net/http/server.go:261 +0xbf
  bufio.(*Writer).Write()
      C:/workdir/go/src/bufio/bufio.go:594 +0x17b
  net/http.(*response).write()
      C:/workdir/go/src/net/http/server.go:1140 +0x316
  net/http.(*response).Write()
      C:/workdir/go/src/net/http/server.go:1112 +0x7f
  (...app...)

Previous write by goroutine 870:
  net/http.(*expectContinueReader).Read()
      C:/workdir/go/src/net/http/server.go:571 +0x351
  (...app...)

@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Oct 26, 2015

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2015

@bradfitz bradfitz self-assigned this Oct 27, 2015

@bradfitz

This comment has been minimized.

Copy link
Member

commented Oct 28, 2015

Unlike the client code, the net/http Server code does not use different goroutines for reading vs. writing.

Your Handler runs in the same goroutine that is reading the request (and request body) and writing the response (and response body).

Can you share your code or a minimal repro?

Are you using TimeoutHandler perhaps?

@linkdata

This comment has been minimized.

Copy link
Author

commented Oct 28, 2015

The handler spawns a goroutine extra in order to handle streaming in a
proxy server, where alternating between reading and writing was difficult.
I realize that this violates the RFCs and I should Hijack() instead of
abusing http.Server. Possibly you might have a situation where a POST is
incoming while an error response is being written from another goroutine,
but that was not my use case. I since have worked around that race.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Oct 28, 2015

We should probably document the status quo for the past 5 years, that http.Handler's ResponseWriter should be written to from the same goroutine that that ServeHTTP was called from.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 17, 2015

I don't think the specific goroutine matters. What matters is that the ResponseWriter must not be written to after the handler returns.

@gopherbot

This comment has been minimized.

Copy link

commented Dec 17, 2015

CL https://golang.org/cl/17982 mentions this issue.

@bradfitz bradfitz closed this in c70df74 Dec 17, 2015

@golang golang locked and limited conversation to collaborators Dec 29, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.