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: should writeLoop flush data after writing the request? #33899

Closed
woodliu opened this issue Aug 28, 2019 · 2 comments
Closed

net/http: should writeLoop flush data after writing the request? #33899

woodliu opened this issue Aug 28, 2019 · 2 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@woodliu
Copy link

woodliu commented Aug 28, 2019

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

1.12.9

$ go version

Does this issue reproduce with the latest release?

Code review. Have question about the code

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

Code review. Have question about the code

go env Output
$ go env

What did you do?

writeLoop run wr.req.Request.write to send request,when it return err==nil it will run pc.bw.Flush() to flush buffered data.
There are two ways for wr.req.Request.write returning nil

  • waitForContinue return nil, the waitForContinue function will be waitForContinue, the only way waitForContinue returns false is when the persistConn close. In this case , there will no way to flush the buffered data

  • bw.Flush() and return nil. So it has flushed the buffered data.(And if bw) is nil, it won‘t need to flush

What did you expect to see?

writeLoop should not run Flush()

What did you see instead?

@bcmills
Copy link
Contributor

bcmills commented Aug 28, 2019

CC @bradfitz @rsc

@bcmills bcmills changed the title src/net/http/transport.go writeLoop shouldn't flush data after write request net/http: should writeLoop flush data after write request? Aug 28, 2019
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 28, 2019
@bcmills bcmills added this to the Go1.14 milestone Aug 28, 2019
@bcmills bcmills changed the title net/http: should writeLoop flush data after write request? net/http: should writeLoop flush data after writing the request? Aug 28, 2019
@bradfitz
Copy link
Contributor

It does this intentionally so requests with a slow/deferred Request.Body don't deadlock. I could dig through the git blame/history to link you to previous bugs with more detail, but you could too, so feel free to do so and add some notes here.

I'm going to close this, as this seemed to just be a question and not a bug report.

@golang golang locked and limited conversation to collaborators Aug 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants