Support graceful connection close

RFC 7540 section 6.8 seems to indicate that traffic can continue flowing in both directions after a GOAWAY frame, with the only restriction being that the recipient can't open new streams after receiving the GOAWAY:

   A GOAWAY frame might not immediately precede closing of the
   connection; a receiver of a GOAWAY that has no more use for the
   connection SHOULD still send a GOAWAY frame before terminating the
   connection.
...
   Activity on streams numbered lower or equal to the last stream
   identifier might still complete successfully.  The sender of a GOAWAY
   frame might gracefully shut down a connection by sending a GOAWAY
   frame, maintaining the connection in an "open" state until all in-
   progress streams complete.

But, h2's connection state machine immediately transitions to CLOSED upon sending or receiving a GOAWAY, effectively prohibiting any further traffic. Would you be interested in a patch to fix this, or is there some disconnect I'm missing between the wording of the RFC and actual implementations?

I'm not familiar enough with the code to know the answer to this, but I'm happy to review a patch if you research it.

Code in question: https://github.com/python-hyper/hyper-h2/blob/570dc7daa480d34bcf0676e88d241112c51b1796/h2/connection.py#L1825-L1844

TL;DR as far as I understand the RFC:

having received GOAWAY with error code 0:
- we can't create more streams
- we are free to send and receive on the existing streams (up to GOAWAY's last stream id)
- higher level library is free to retry requests with higher stream ids (peer either never saw them, or never parsed them)
having receive GOAWAY with non-zero error:
- we can't create new streams
- we can't send more data on existing streams
- we should probably keep the data already received for our user
- we should treat this as ConnectionError and [dunno about response] and close the connection

Cross-link: https://github.com/encode/httpx/issues/828

Would it help to split the input two ways:

RECV_GOAWAY for errors error_code != 0
RECV_GRACEFUL_SHUTDOWN or RECV_LAST_STREAM_ID for error_code == 0?

+1 on this issue
Since such software like HAProxy terminates connections by sending GOAWAY first and HEADERS/DATA frames right after - the current transition states are unable to handle this.

Any updates on this?

I've looked at fixing this and it seems like it'd be a lot of work. I'm very hesitant to dig into it without guidance from maintainers. It seems this is further complicated by two-stage GOAWAY handling but that does not appear to be well supported in general, for example https://mailman.nginx.org/pipermail/nginx-devel/2021-April/013956.html.

@Kriechi

Coming at this from mitmproxy, where it turns up as mitmproxy/mitmproxy#7879. A fair number of sites, or really the load balancer in front of them, send a graceful GOAWAY and then finish the response they'd already started. h2 raises Invalid input ConnectionInputs.RECV_DATA in state ConnectionState.CLOSED at that point and the request dies.

I read back through the thread and dimaqq's summary of the semantics matches my reading of RFC 9113, so I had a go at implementing it. I've got something working locally, suite passes, added tests for the draining cases. But I'm not confident I got the design right, and since the design looks like where this stalled before, I'd rather ask than open a PR that goes nowhere.

The part I'm unsure about is that I took a shortcut. Instead of adding a RECV_GRACEFUL_SHUTDOWN input and a draining state like dimaqq suggested, I left the connection in CLOSED and recorded the boundary on the state machine, then let DATA/HEADERS/WINDOW_UPDATE/RST_STREAM through the KeyError path when the stream id is at or below it. My reasoning was that it wouldn't disturb anything downstream checking state is CLOSED.

Then I actually went and looked, and that reasoning might be backwards. fetchling has while connection.state_machine.state is not ConnectionState.CLOSED around its read loop, so with my version it stops reading on a graceful GOAWAY even though h2 would accept the frames. Those consumers might genuinely need a way to tell draining apart from dead, which argues for doing it with a real state instead. I don't know this codebase's history well enough to call it.

Two smaller things I'd value an opinion on. process_input doesn't take a stream id today and mine needs one to apply the boundary, so stream context leaks into what the docs describe as a deliberately coarse, high-level connection machine. And on two-stage GOAWAY, keeping the smallest last_stream_id seen made it fall out for free, but zanieb's nginx link suggests it isn't widely supported, so I'm not sure it's worth designing for.

So how would you want this done? Happy to put the work in either way, and happy to be told the shortcut is wrong.

One thing I noticed while testing, in case it's useful when you're deciding: #1324 drops every non-GOAWAY frame once the connection is CLOSED, which stops the exception but the in-flight body goes with it. Ran the mitmproxy repro against it and got a 200 with an empty body. That one targets #1199 though, so it isn't trying to cover this.

@Kriechi sorry to pull you into a seven year old issue nobody's touched since 2022. Mainly after a steer on direction before I write anything up.

Navigation Menu

Repository navigation

Support graceful connection close #1181

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Support graceful connection close #1181

Description

Activity

pgjones commented on Jan 24, 2019

dimaqq commented on Feb 28, 2020

dimaqq commented on Apr 1, 2020

ech0-py commented on Mar 8, 2021

stephenc-pace commented on Nov 18, 2022

zanieb commented on Nov 30, 2022

neoLsH commented on Oct 1, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions