diff --git a/src/httpcore2/httpcore2/_async/http2.py b/src/httpcore2/httpcore2/_async/http2.py index 7d2dab5d..0022a2b6 100644 --- a/src/httpcore2/httpcore2/_async/http2.py +++ b/src/httpcore2/httpcore2/_async/http2.py @@ -373,9 +373,10 @@ async def _receive_remote_settings_change(self, event: h2.events.RemoteSettingsC self._max_streams -= 1 async def _response_closed(self, stream_id: int) -> None: - await self._max_streams_semaphore.release() async with self._state_lock: - del self._events[stream_id] + if stream_id in self._events: + await self._max_streams_semaphore.release() + del self._events[stream_id] if self._connection_terminated and not self._events: await self.aclose() diff --git a/src/httpcore2/httpcore2/_sync/http2.py b/src/httpcore2/httpcore2/_sync/http2.py index 992b42ce..3b79af8c 100644 --- a/src/httpcore2/httpcore2/_sync/http2.py +++ b/src/httpcore2/httpcore2/_sync/http2.py @@ -373,9 +373,10 @@ def _receive_remote_settings_change(self, event: h2.events.RemoteSettingsChanged self._max_streams -= 1 def _response_closed(self, stream_id: int) -> None: - self._max_streams_semaphore.release() with self._state_lock: - del self._events[stream_id] + if stream_id in self._events: + self._max_streams_semaphore.release() + del self._events[stream_id] if self._connection_terminated and not self._events: self.close() diff --git a/tests/httpcore2/_async/test_http2.py b/tests/httpcore2/_async/test_http2.py index 48dbef3b..5bed947f 100644 --- a/tests/httpcore2/_async/test_http2.py +++ b/tests/httpcore2/_async/test_http2.py @@ -68,6 +68,37 @@ async def test_http2_connection_closed() -> None: assert not conn.is_available() +@pytest.mark.anyio +async def test_http2_response_closed_twice() -> None: + """ + Closing a response for a stream that has already been removed should be + a no-op, rather than raising a `KeyError` that masks the exception which + triggered the cleanup. See https://github.com/encode/httpx/issues/3072 + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + hyperframe.frame.SettingsFrame().serialize(), + hyperframe.frame.HeadersFrame( + stream_id=1, + data=hpack.Encoder().encode( + [ + (b":status", b"200"), + (b"content-type", b"plain/text"), + ] + ), + flags=["END_HEADERS"], + ).serialize(), + hyperframe.frame.DataFrame(stream_id=1, data=b"Hello, world!", flags=["END_STREAM"]).serialize(), + ] + ) + async with httpcore2.AsyncHTTP2Connection(origin=origin, stream=stream, keepalive_expiry=5.0) as conn: + await conn.request("GET", "https://example.com/") + + # The stream was closed when the response completed. + await conn._response_closed(stream_id=1) + + @pytest.mark.anyio async def test_http2_connection_post_request() -> None: origin = httpcore2.Origin(b"https", b"example.com", 443) diff --git a/tests/httpcore2/_sync/test_http2.py b/tests/httpcore2/_sync/test_http2.py index 3b37a160..cfdc1f9e 100644 --- a/tests/httpcore2/_sync/test_http2.py +++ b/tests/httpcore2/_sync/test_http2.py @@ -69,6 +69,37 @@ def test_http2_connection_closed() -> None: +def test_http2_response_closed_twice() -> None: + """ + Closing a response for a stream that has already been removed should be + a no-op, rather than raising a `KeyError` that masks the exception which + triggered the cleanup. See https://github.com/encode/httpx/issues/3072 + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + hyperframe.frame.SettingsFrame().serialize(), + hyperframe.frame.HeadersFrame( + stream_id=1, + data=hpack.Encoder().encode( + [ + (b":status", b"200"), + (b"content-type", b"plain/text"), + ] + ), + flags=["END_HEADERS"], + ).serialize(), + hyperframe.frame.DataFrame(stream_id=1, data=b"Hello, world!", flags=["END_STREAM"]).serialize(), + ] + ) + with httpcore2.HTTP2Connection(origin=origin, stream=stream, keepalive_expiry=5.0) as conn: + conn.request("GET", "https://example.com/") + + # The stream was closed when the response completed. + conn._response_closed(stream_id=1) + + + def test_http2_connection_post_request() -> None: origin = httpcore2.Origin(b"https", b"example.com", 443) stream = httpcore2.MockStream(