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

Handle half closed per HTTPWG recommendation #1264

Open
2 tasks done
pgjones opened this issue Nov 24, 2021 · 2 comments
Open
2 tasks done

Handle half closed per HTTPWG recommendation #1264

pgjones opened this issue Nov 24, 2021 · 2 comments
Assignees
Labels

Comments

@pgjones
Copy link
Contributor

pgjones commented Nov 24, 2021

Checklist

  • The bug is reproducible against the latest release and/or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

I've been investigating this bug report in Hypercorn and noticed that uvicorn considers half closed connections as indicating that the client is no longer interested in the response, rather than that the client has no interest in sending more. This has been recently discussed in the HTTPWG with this clarification and as I understand it Uvicorn should not act as it does. (Hypercorn follows the recommendation which partially explains the difference in the initial bug report).

To reproduce

Have Uvicorn serve,

from asyncio import CancelledError, sleep

from quart import Quart

app = Quart(__name__)

@app.route("/")
async def main():
    print("START")
    try:
        await sleep(10)
    except CancelledError:
        print("CANCELLED")
    finally:
        print("END")
    return "Hello"

and then run the following,

socket_path = ("localhost", 8000)

import socket
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.connect(socket_path)

# Write command to socket
s.send(b"GET / HTTP/1.1\r\nhost: abcd\r\n\r\n")

# Important: Close sending direction. That way
# the other side knows we are finished.
s.shutdown(socket.SHUT_WR)

# Now read the answer
answer = s.recv(100000000)

# Parse the answer into a table (a list of lists)
table = [ line.split(b';') for line in answer.split(b'\n')[:-1] ]

print(table)

Expected behavior

A response is printed.

Actual behavior

Uvicorn doesn't serve a response

Environment

Running uvicorn 0.15.0 with CPython 3.9.5 on Darwin

Important

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@Tinche
Copy link

Tinche commented Nov 24, 2021

I'm the author of the mentioned Hypercorn bug report. Googling around it seems there are basically two ways of dropping a TCP connection:

  • closing it on one end (putting it in a half-closed state temporarily)
  • sending an RST and just abandoning the connection

My initial repro case was simply hitting the service with curl and killing curl shortly thereafter. I don't actually know what happens on the wire in this case, but it's kind of a synthetic case anyway.

The case that is not synthetic and actually matters in the real world would be a timeout happening on the client end, so I would be curious to know what httpx (and maybe some other clients) do in this situation.

On a high level, when a timeout happens on the client end generally the client is not interested in the response. I want to be aware of this on the server end ASAP so I can stop further processing and release resources immediately (like freeing up space in various connection pools and letting the database know it can stop processing the query). I think in the real world a case like this is most likely to happen if you hit an endpoint that runs an inefficient database query (f.e. no index); if you don't stop running the query when the client gives up you'll just murder your database.

Maybe the transport layer is the wrong layer to communicate this information. That would be understandable but unfortunate. The current Uvicorn cancellation handling is quite useful.

@irvingreid
Copy link

Slid over here from the HTTPWG comment thread, to note that HTTP 1.0 did (arguably) specify that when either side half-closes, the server should stop processing the request: the last paragraph of https://www.rfc-editor.org/rfc/rfc1945#section-1.3, two pages down, just above https://www.rfc-editor.org/rfc/rfc1945#section-1.4.

HTTP 1.1 doesn't explicitly change this behaviour, but the general rules around closing and reconnecting connections in the presence of keepalive don't have anything that requires the server to react to a half-close. The sad truth is that HTTP does not (to the best of my knowledge) standardize a way to tell a server to abandon processing on a request.

It would be perfectly reasonable for a server to abandon processing when it receives a RST before (or while) sending its response; at that point, the standard is clear that the client cannot know whether or not any side effects of the request have been performed, and any response data generated by the server is going to be thrown away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants