Skip to content

Conversation

@wendigo
Copy link
Contributor

@wendigo wendigo commented Jun 7, 2024

With the current Jersey and Jetty implementations @suspended AsyncResponse can generate a lot of warnings that are caused by the fact that Jetty eagerly recycles request/response objects when the HTTP/2 RST_STREAM frame is sent by the client during request aborting.

We abort in-flight requests for different reasons: either timeout is reached or when the client is closed during cleanup.

For HTTP/1 that doesn't matter, but for HTTP/2 this will cause the AsyncResponse.resume to log an error, since the underlying request/response objects are already recycled and there is no client listening for the response.

This change also cancels Future<?> bound to the AsyncContext since there is no point in processing it anymore.

Description

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

With the current Jersey and Jetty implementations @suspended AsyncResponse
can generate a lot of warnings that are caused by the fact that Jetty eagerly recycles
request/response objects when the HTTP/2 RST_STREAM frame is sent by the client during request aborting.

We abort in-flight requests for different reasons: either timeout is reached or when the client is closed during cleanup.

For HTTP/1 that doesn't matter, but for HTTP/2 this will cause the AsyncResponse.resume to log an error,
since the underlying request/response objects are already recycled and there is no client listening for the response.

This change also cancels Future<?> bound to the AsyncContext since there is no point in processing it anymore.
@wendigo wendigo requested review from arhimondr, dain and electrum June 7, 2024 11:54
@cla-bot cla-bot bot added the cla-signed label Jun 7, 2024
@wendigo
Copy link
Contributor Author

wendigo commented Jun 7, 2024

Thanks @dain @electrum for the ideas.

@wendigo
Copy link
Contributor Author

wendigo commented Jun 7, 2024

@electrum @dain this replaces airlift/airlift#1169

@wendigo wendigo closed this Jun 21, 2024
@wendigo wendigo deleted the serafin/async-response-client-disconnected branch June 27, 2024 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants