Skip to content

Ensure h2 connection gets closed in case of exception#253

Merged
lovelydinosaur merged 4 commits into
masterfrom
fm/http2-close-conn
Nov 18, 2021
Merged

Ensure h2 connection gets closed in case of exception#253
lovelydinosaur merged 4 commits into
masterfrom
fm/http2-close-conn

Conversation

@florimondmanca

Copy link
Copy Markdown
Contributor

May close encode/httpx#1414

Tweaks our HTTP/2 connection implementation so that the underlying h2_state is properly closed before bubbling up any exceptions.

Right now it seems possible for the underlying state to leak eg if the connection gets lost unexpectedly — see: https://github.com/encode/httpx/issues/1414#issuecomment-742127990

@florimondmanca florimondmanca changed the title Ensure h2 connection gets closed Ensure h2 connection gets closed in case of exception Dec 12, 2020
@frankwalter1301

frankwalter1301 commented Nov 13, 2021

Copy link
Copy Markdown

Doesn't work with the new version of httpcore. 'AsyncHTTP2Connection' object has no attribute 'h2_state' By changing h2_state to _h2_state in respect to the new naming of the new releases I get another error: Invalid input ConnectionInputs.SEND_HEADERS in state ConnectionState.CLOSED

@lovelydinosaur

Copy link
Copy Markdown
Contributor

@frankwalter1301 Could you give some more background about why you're bumping into this?

@frankwalter1301

frankwalter1301 commented Nov 16, 2021 via email

Copy link
Copy Markdown

@lovelydinosaur

Copy link
Copy Markdown
Contributor

Believe that #440 is what we need to close encode/httpx#1414, but yes! this is a good and sensible bit of defensive programming, and we should have it in.

@lovelydinosaur lovelydinosaur merged commit de62932 into master Nov 18, 2021
@lovelydinosaur lovelydinosaur deleted the fm/http2-close-conn branch November 18, 2021 15:32
@lovelydinosaur lovelydinosaur mentioned this pull request Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Max outbound streams is 100, 100 open

3 participants