Skip to content

Release HTTP/2 semaphore permit on NoAvailableStreamIDError#1012

Merged
Kludex merged 1 commit into
pydantic:mainfrom
mbeijen:fix/h2-release-semaphore-on-no-stream-id
Jun 2, 2026
Merged

Release HTTP/2 semaphore permit on NoAvailableStreamIDError#1012
Kludex merged 1 commit into
pydantic:mainfrom
mbeijen:fix/h2-release-semaphore-on-no-stream-id

Conversation

@mbeijen
Copy link
Copy Markdown
Contributor

@mbeijen mbeijen commented Jun 2, 2026

Summary

AsyncHTTP2Connection.handle_async_request acquires a permit from _max_streams_semaphore before calling self._h2_state.get_next_available_stream_id(). If the stream-ID space is exhausted and NoAvailableStreamIDError is raised, the exception handler sets _used_all_stream_ids = True and decrements _request_count but never releases the permit — leaking it permanently.

In practice _used_all_stream_ids = True prevents further requests on the same connection so the visible impact is limited, but the resource cleanup is still wrong.

This PR adds the missing release() call in both the async and sync paths.

Ports encode/httpcore#1061 (bysiber, currently open upstream), via codeberg.org/httpxyz/httpcorexyz@e88f30b. The fork bundled this fix with encode/httpcore#1062; I'm splitting them into two httpx2 PRs to mirror the upstream split.

Notes

  • Only _async/http2.py was edited by hand; _sync/http2.py was regenerated by scripts/unasync.py.
  • No new test: the NoAvailableStreamIDError branch is already marked # pragma: no cover because exhausting the H/2 client stream-ID space (≈ 2³⁰ requests) is impractical to exercise in a unit test. The upstream PR doesn't ship a test either. 100% coverage preserved.

Note: this change was prepared with AI assistance (Claude Code).

`handle_async_request` / `handle_request` acquires a permit from
`_max_streams_semaphore` before calling
`_h2_state.get_next_available_stream_id()`. If the stream-ID space is
exhausted and `NoAvailableStreamIDError` is raised, the exception
handler sets `_used_all_stream_ids` and decrements `_request_count`
but never releases the permit — leaking it permanently.

In practice `_used_all_stream_ids = True` prevents further requests on
the connection, so the visible impact is limited, but the resource
cleanup is still wrong.

Add the missing `release()` call in both async and sync paths.

Ports encode/httpcore#1061 (bysiber), via
codeberg.org/httpxyz/httpcorexyz@e88f30b (first of two fixes bundled
in that commit; the second lands in a separate PR).

Co-Authored-By: Kadir Can Ozden <101993364+bysiber@users.noreply.github.com>
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 2, 2026

Merging this PR will not alter performance

✅ 15 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing mbeijen:fix/h2-release-semaphore-on-no-stream-id (c2a2cfb) with main (fe3da36)

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

mbeijen added a commit to mbeijen/httpx2 that referenced this pull request Jun 2, 2026
@Kludex Kludex merged commit b776ea9 into pydantic:main Jun 2, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants