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.

Sign up for GitHub

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: Client / Transport deadlock #2616

Closed
gopherbot opened this issue Dec 25, 2011 · 13 comments
Closed

net/http: Client / Transport deadlock #2616

gopherbot opened this issue Dec 25, 2011 · 13 comments
Labels
FrozenDueToAge
Milestone
Go1

Comments

@gopherbot
Copy link

by yoshiyuki.kanno@stoic.co.jp:

What steps will reproduce the problem?
1. Invoke massive http requests by http.(Get|Head|...) to a same domain(for using pconn).

2. When switching goroutine between "http.(*persistConn).expectingResponse()"
and "http.(*persistConn).close()" in http.readLoop, scheduled goroutine
invoking http requests  will get into dead locked in
"http.(*persistConn).roundTrip". because  "responseAndError" will
never get data.

What is the expected output?
http.(*Transport).RoundTrip should return with an appropriate error.

What do you see instead?
To prevent getting into this dead locked, the critical region of
"http.(*persistConn).expectingResponse()" should include
"http.(*persistConn).close()".


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

Which operating system are you using?
Linux ubuntu11 3.0.0-12-server x86_64
Linux CentOS6 2.6.18-238.12.1.el5 x86_64

Which revision are you using?  (hg identify)
c1702f36df03 (release-branch.r60) release/release.r60.3
8e23f5bdc859 tip

Please provide any additional information below.
attached a quick patch for r60.3

Attachments:

  1. transport.patch (3956 bytes)
@gopherbot
Copy link
Author

Comment 1 by nekotaroh:

this issue can't be fixed completely without fixing issue #2645. because can not
detect`pc.cc.Write(req)`'s error in (*persistConn).roundTrip.
For details, please refer to issue #2645

@gopherbot
Copy link
Author

Comment 2 by nekotaroh:

published reproducible codes on github.
https://github.com/mocchira/digestw
and the following code is where Getting into dead locked.
https://github.com/mocchira/digestw/blob/master/common/http.go
After applied patches issue #2616(this) and issue #2645,
No dead locked happen for now.

@bradfitz
Copy link
Contributor

bradfitz commented Jan 9, 2012

Comment 3:

We don't submit patches over the bug tracker.
To submit code changes, see:
http://golang.org/doc/contribute.html
Notably,
1) you need to submuit a CLA (http://golang.org/doc/contribute.html#copyright)
2) you need to use the codereview site.
Alternatively, I could work on fixing this independently, but I'd prefer to take your
change if you want to do the steps above.

Owner changed to @bradfitz.

Status changed to Accepted.

@gopherbot
Copy link
Author

Comment 4 by nekotaroh:

submitted patches.
http://golang.org/cl/5532057/

@bradfitz
Copy link
Contributor

Comment 5:

You haven't submitted a CLA that I can find.
See: http://golang.org/doc/contribute.html#copyright

@gopherbot
Copy link
Author

Comment 6 by nekotaroh:

hmm.. I had done it through online, but might be migging something else.
So I'll do it again.
Thanks

@gopherbot
Copy link
Author

Comment 7 by nekotaroh:

submitted two CLAs with yoshiyuki.kanno@stoic.co.jp and nekotaroh@gmail.com

@bradfitz
Copy link
Contributor

Comment 8:

Issue #2645 has been merged into this issue.

@robpike
Copy link
Contributor

robpike commented Jan 13, 2012

Comment 10:

Labels changed: added priority-go1.

@robpike
Copy link
Contributor

robpike commented Jan 13, 2012

Comment 11:

Owner changed to builder@golang.org.

@bradfitz
Copy link
Contributor

Comment 12:

Fix for this is in review. Waiting on one more round.

Owner changed to @bradfitz.

Status changed to Started.

@bradfitz
Copy link
Contributor

Comment 14:

The latest version of the CL doesn't pass its new test on my machine:
$ go test -v net/http
--- FAIL: TestClientTransportDeadlock (30.00 seconds)
        client_test.go:486: probably dead locked at [7830 7883 7886 7890 7935 7936 7912 7902]
... also doesn't fail with a great error message.

@bradfitz
Copy link
Contributor

Comment 15:

This issue was closed by revision d645adc.

Status changed to Fixed.

@bradfitz bradfitz mentioned this issue Dec 8, 2014
Closed
@rsc rsc added this to the Go1 milestone Apr 10, 2015
@rsc rsc removed the priority-go1 label Apr 10, 2015
@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
Go1
Development

No branches or pull requests
4 participants
@bradfitz @rsc @robpike @gopherbot