uvicorn fix: keep waiting if connection is not ready to close

related to https://github.com/encode/uvicorn/issues/111

Sep 07 '22 16:09 Yangruipis

I need more information here...

Sep 17 '22 20:09 Kludex

Here is the way to reproduce this problem.

Sep 18 '22 15:09 Yangruipis

If you can explain the problem, and the solution, it would save me time, and I'd really appreciate it.

Sep 18 '22 15:09 Kludex

If you can explain the problem, and the solution, it would save me time, and I'd really appreciate it.

Sorry for my late response.

Problem

The close of the connection after timeout_keep_alive may raise error like that:

h11._util.LocalProtocolError: can't handle event type ConnectionClosed when role=SERVER and state=SEND_RESPONSE

Because the function timeout_keep_alive_handler is called when the connection is still sending data. This may happen with a very little probability under high concurrency, like a locust test.

https://github.com/encode/uvicorn/blob/c3c2db3f9ed8e331c6757c9c302ef21cf7338848/uvicorn/protocols/http/h11_impl.py#L352-L360

Solution

if the connection is still sending data, do not close it
(optional) put it back to the loop, which will be called after another timeout. Or we do nothing until another timeout_keep_alive_task is launched on response complete.

https://github.com/encode/uvicorn/blob/47475dd30e3801c5ebbdf00087123a54ab46e8e0/uvicorn/protocols/http/h11_impl.py#L352-L366

Oct 04 '22 16:10 Yangruipis

I cannot reproduce your example. Are you able to create a test case on this PR?

Oct 30 '22 07:10 Kludex

I cannot reproduce your example. Are you able to create a test case on this PR?

I have added a new case and some comments. But it's interesting that the new case passed locally, but failed in CI. I'll check my local package version firstly, and reopen this PR if I can make sure about it.

However, if you want to reproduce this issue, just run the new test case with the master branch code. It will raise error like that:

Oct 31 '22 17:10 Yangruipis