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: An empty reply causes false race conditions (go1.3beta1+) #7856

Closed
gopherbot opened this issue Apr 24, 2014 · 6 comments
Closed

net/http: An empty reply causes false race conditions (go1.3beta1+) #7856

gopherbot opened this issue Apr 24, 2014 · 6 comments
Milestone

Comments

@gopherbot
Copy link

by travis.bischel:

What does 'go version' print?
go version devel +f8b50ad4cac4 Mon Apr 21 17:00:27 2014 -0700 + linux/amd64

What steps reproduce the problem?
If possible, include a link to a program on play.golang.org.

This can be done with three files:
http://play.golang.org/p/bxMjv7H1cw
1. create server.go and `go run` it
2. create race.go and race_test.go in a different directory
3. go test -race

What happened?
$ go test -race
==================
WARNING: DATA RACE
Write by goroutine 6:
  strings.(*Reader).Read()
      /usr/local/go/src/pkg/strings/reader.go:32 +0x6f
  go.(*struct { *strings.Reader; io.Closer }).Read()
      /usr/local/go/src/pkg/net/http/chunked.go:1 +0x90
  net/http.(*bodyEOFSignal).Read()
      /usr/local/go/src/pkg/net/http/transport.go:1123 +0x278
  io/ioutil.devNull.ReadFrom()
      /usr/local/go/src/pkg/io/ioutil/ioutil.go:151 +0xda
  io.Copy()
      /usr/local/go/src/pkg/io/io.go:349 +0x156
  _/home/twmb/testing.request()
      /home/twmb/testing/race.go:16 +0x1aa

Previous write by goroutine 7:
  strings.(*Reader).Read()
      /usr/local/go/src/pkg/strings/reader.go:32 +0x6f
  go.(*struct { *strings.Reader; io.Closer }).Read()
      /usr/local/go/src/pkg/net/http/chunked.go:1 +0x90
  net/http.(*bodyEOFSignal).Read()
      /usr/local/go/src/pkg/net/http/transport.go:1123 +0x278
  io/ioutil.devNull.ReadFrom()
      /usr/local/go/src/pkg/io/ioutil/ioutil.go:151 +0xda
  io.Copy()
      /usr/local/go/src/pkg/io/io.go:349 +0x156
  _/home/twmb/testing.request()
      /home/twmb/testing/race.go:16 +0x1aa

Goroutine 6 (running) created at:
  _/home/twmb/testing.RaceTest()
      /home/twmb/testing/race.go:28 +0x94
  _/home/twmb/testing.TestRaceTest()
      /home/twmb/testing/race_test.go:8 +0x2b
  testing.tRunner()
      /usr/local/go/src/pkg/testing/testing.go:422 +0x10f

Goroutine 7 (finished) created at:
  _/home/twmb/testing.RaceTest()
      /home/twmb/testing/race.go:29 +0xc3
  _/home/twmb/testing.TestRaceTest()
      /home/twmb/testing/race_test.go:8 +0x2b
  testing.tRunner()
      /usr/local/go/src/pkg/testing/testing.go:422 +0x10f
==================
PASS
Found 1 data race(s)
exit status 66
FAIL    _/home/twmb/testing 1.028s

What should have happened instead?
No race condition.

Please provide any additional information below.
https://code.google.com/p/go/source/browse/src/pkg/net/http/response.go#233
https://code.google.com/p/go/source/browse/src/pkg/net/http/server.go#1977
@gopherbot
Copy link
Author

Comment 1 by travis.bischel:

This is specifically only for empty responses.

@ianlancetaylor
Copy link
Contributor

Comment 2:

Labels changed: added repo-main, release-go1.3maybe.

@dvyukov
Copy link
Member

dvyukov commented Apr 24, 2014

Comment 3:

Labels changed: added threadsanitizer.

@bradfitz
Copy link
Contributor

Comment 4:

This is a regression from https://code.google.com/p/go/source/detail?r=358e2b416518#
The Go http package contains this Body ReadCloser:
// eofReader is a non-nil io.ReadCloser that always returns EOF.
// It embeds a *strings.Reader so it still has a WriteTo method
// and io.Copy won't need a buffer.
var eofReader = &struct {
    *strings.Reader
    io.Closer
}{
    strings.NewReader(""),
    ioutil.NopCloser(nil),
}
That used to be safe to read from by multiple goroutines. Now that always-EOF Read is
also a write.

Owner changed to @bradfitz.

Status changed to Accepted.

@gopherbot
Copy link
Author

Comment 5:

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

@bradfitz
Copy link
Contributor

Comment 6:

This issue was closed by revision 13ea1fd.

Status changed to Fixed.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants