Skip to content
This repository has been archived by the owner. It is now read-only.

Don't wait to consume the entire request body for Connection: close requests #406

Closed
halter73 opened this issue Nov 19, 2015 · 6 comments
Closed
Assignees
Labels
Milestone

Comments

@halter73
Copy link
Member

@halter73 halter73 commented Nov 19, 2015

If you hit a sample app running on Kestrel with ab (apache bench which makes HTTP/1.0 requests), every request times out.

This is caused by Kestrel becoming stuck on await messageBody.Consume() in Frame.RequestProcessingAsync.

I think we fix this by simply not consuming the rest of the request for non keep-alive connections.

@halter73 halter73 added the bug label Nov 19, 2015
@halter73 halter73 added this to the 1.0.0-rc2 milestone Nov 19, 2015
@benaadams
Copy link
Contributor

@benaadams benaadams commented Nov 19, 2015

Was putting a PR together for this but it needs to go after "Precomputed header bytes" #367 if that's happening as its all change

@cesarblum
Copy link
Contributor

@cesarblum cesarblum commented Nov 19, 2015

@benaadams The fix is simple. I was thinking something like:

if (_keepAlive)
{
    // Finish reading the request body in case the app did not.
    await messageBody.Consume();
}

Is this what you were thinking? The only reason this isn't in a PR yet is we need a regression test.

@benaadams
Copy link
Contributor

@benaadams benaadams commented Nov 20, 2015

@CesarBS yes, good point :)

@cesarblum
Copy link
Contributor

@cesarblum cesarblum commented Nov 21, 2015

Closed by e4fd91b.

@amcdnl
Copy link

@amcdnl amcdnl commented Feb 17, 2016

@cesarblum
Copy link
Contributor

@cesarblum cesarblum commented Feb 17, 2016

@amcdnl Looks like it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants