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

Jump to bottom

net/http/httputil: unit test failure #3892

Closed
alberts opened this issue Aug 1, 2012 · 12 comments
Closed

net/http/httputil: unit test failure #3892

alberts opened this issue Aug 1, 2012 · 12 comments
Labels
FrozenDueToAge

Comments

@alberts
Copy link
Contributor

alberts commented Aug 1, 2012 

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
1. run net/http/httputil unit tests repeatedly

What do you see instead?

=== RUN TestChunk-81
--- PASS: TestChunk-81 (0.00 seconds)
=== RUN TestDumpRequest-81
--- FAIL: TestDumpRequest-81 (0.00 seconds)
dump_test.go:127:   DumpRequestOut 3, expecting:
        GET /foo HTTP/1.1
        Host: example.com
        User-Agent: Go http package
        Accept-Encoding: gzip
        
        
        Got:
        
        === RUN TestReverseProxy-81
--- PASS: TestReverseProxy-81 (0.00 seconds)
=== RUN TestXForwardedFor-81
--- PASS: TestXForwardedFor-81 (0.00 seconds)
=== RUN TestReverseProxyQuery-81
--- PASS: TestReverseProxyQuery-81 (0.00 seconds)
=== RUN TestReverseProxyFlushInterval-81
--- PASS: TestReverseProxyFlushInterval-81 (0.00 seconds)
FAIL

Which compiler are you using (5g, 6g, 8g, gccgo)?

6g

Which operating system are you using?

linux

Which version are you using?  (run 'go version')

go version weekly.2012-03-27 +aec2d2eec7cf

GOMAXPROCS=81
@alberts
Copy link
Contributor Author

alberts commented Aug 17, 2012

Comment 1:

Still seeing this at tip.
go version weekly.2012-03-27 +e2f74da67564

@davecheney
Copy link
Contributor

Comment 2:

Dmitry: is threadsanitizer able to give any hits about what is broken here? I can
readily reproduce this breakage at GOMAXPROCS=81, even on a dual core arm pandaboard!

Labels changed: added priority-soon, removed priority-triage.

Owner changed to @davecheney.

Status changed to Accepted.

@alberts
Copy link
Contributor Author

alberts commented Aug 22, 2012

Comment 3:

Also saw this today:
--- FAIL: TestReverseProxyQuery-78 (0.00 seconds)
reverseproxy_test.go:147: 3. got query ""; expected "sta=tic"

@dvyukov
Copy link
Member

dvyukov commented Aug 23, 2012

Comment 4:

I've already reported all data races that I've seen on std lib tests and benchmarks.
ThreadSanitizer can catch only low-level data race, it can't catch e.g. high-level
logical races.

@dvyukov
Copy link
Member

dvyukov commented Aug 23, 2012

Comment 5:

Run once again:
$ go test -v -bench=.* -race -cpu=1,2,4,8,16,32,64,128,81,1,16 net/http/httputil
no races

@davecheney
Copy link
Contributor

Comment 6:

I can reproduce the failure on arm and amd64 with GOMAXPROCS=81, is
this somehow different to -cpu=81?

@dvyukov
Copy link
Member

dvyukov commented Aug 23, 2012

Comment 7:

GOMAXPROCS and -cpu are mostly the same wrt thread execution.
The difference is that -cpu=X sets GOMAXPROCS=X just before test execution and resets it
back to 1 after the tests.

@dvyukov
Copy link
Member

dvyukov commented Aug 23, 2012

Comment 8:

I can't reproduce the failure under tsan, most likely it has to do with disturbed
timings under tsan.

@davecheney
Copy link
Contributor

Comment 9:

Hello,
Could you please try this CL
http://golang.org/cl/6483061
Cheers
Dave

@alberts
Copy link
Contributor Author

alberts commented Aug 27, 2012

Comment 10:

c552fb2b6a6c+ tip
issue6483061_2001.diff applied
=== RUN TestReverseProxyFlushInterval-59
--- FAIL: TestReverseProxyFlushInterval-59 (5.00 seconds)
reverseproxy_test.go:191: maxLatencyWriter flushLoop() never exited
FAIL

@davecheney
Copy link
Contributor

Comment 11:

I think that one is a genuine data race, the onExitFlushLoop function
pointer is mutated by several goroutines without a fence. I will open
a new issue.

@davecheney
Copy link
Contributor

Comment 12:

This issue was closed by revision f8d4bb8.

Status changed to Fixed.

davecheney added a commit that referenced this issue May 11, 2015
@davecheney
[release-branch.go1] net/http/httputil: fix race in DumpRequestOut
aec9af0 
««« backport 3b78b41a4b50
net/http/httputil: fix race in DumpRequestOut

Fixes #3892.

Swapping the order of the writers inside the MultiWriter ensures
the request will be written to buf before http.ReadRequest completes.

The fencedBuffer is not required to make the test pass on
any machine that I have access too, but as the buf is shared
across goroutines, I think it is necessary for correctness.

R=bradfitz, fullung, franciscossouza
CC=golang-dev
https://golang.org/cl/6483061

»»»
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@davecheney @alberts @dvyukov @gopherbot