Skip to content

Recycle HTTP client lease when an unsent request is reset or cancelled - #6468

Open
sriggin wants to merge 2 commits into
eclipse-vertx:masterfrom
sriggin:http-stream-fix
Open

sriggin wants to merge 2 commits into
eclipse-vertx:masterfrom
sriggin:http-stream-fix

Conversation

@sriggin

@sriggin sriggin commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Resetting or cancelling an HttpClientRequest before anything was written did not close the stream on HTTP/2 and HTTP/3, so the connection lease was never recycled. Each such request permanently consumed a concurrency slot; once exhausted, new requests waited in the pool until the connection closed. Neither the idle timeout nor the keep-alive expiry reclaims these slots.

  • HTTP/2: close the stream when it is reset before stream allocation.
  • HTTP/3: when the request headers were not sent, also abort the inbound side and close the stream channel, since Netty does not close it after a reset.

Add reset and cancel tests covering all HTTP versions.

Motivation:

We have a use-case where acquiring a connection/stream from the pool that takes time (say connection establishment) exceeds a dynamic latency budget and we want to release the connection/stream lease without using it. cancel() appears to be the best fit but this bug prevents us from using it.

Assisted-by: Anthropic Claude (claude-opus-5-5)

Resetting or cancelling an HttpClientRequest before anything was written
did not close the stream on HTTP/2 and HTTP/3, so the connection lease was
never recycled. Each such request permanently consumed a concurrency slot;
once exhausted, new requests waited in the pool until the connection
closed. Neither the idle timeout nor the keep-alive expiry reclaims these
slots.

- HTTP/2: close the stream when it is reset before stream allocation.
- HTTP/3: when the request headers were not sent, also abort the inbound
  side and close the stream channel, since Netty does not close it after
  a reset.

Add reset and cancel tests covering all HTTP versions.

Assisted-by: Anthropic Claude (claude-opus-5-5)
Signed-off-by: Sean Riggin <sriggin@gmail.com>
@vietj

vietj commented Oct 27, 2026

Copy link
Copy Markdown
Member

it seems your contribution has issues with CI, you should check the log and assess locally, perhaps it is regression or perhaps the tests are incorrectly written and do not pass anymore

@sriggin

sriggin commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

it seems your contribution has issues with CI, you should check the log and assess locally, perhaps it is regression or perhaps the tests are incorrectly written and do not pass anymore

Yup, sorry! I missed running the broader test suite locally, which showed that another small change (2nd commit) was necessary for the close handler.

@sriggin

sriggin commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@vietj This should be ready to go now, my miss in other tests has been addressed

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