Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Serialize HttpClient request failures #11037

Closed
sbordet opened this issue Dec 11, 2023 · 1 comment · Fixed by #11038
Closed

Serialize HttpClient request failures #11037

sbordet opened this issue Dec 11, 2023 · 1 comment · Fixed by #11038
Assignees
@sbordet
Labels
Bug For general bugs on Jetty side

Comments

@sbordet
Copy link
Contributor

sbordet commented Dec 11, 2023

Jetty version(s)
12

Description
When a request is being sent, it may be asynchronously aborted.

The current code, upon request abort from thread T1, calls HttpSender.terminateRequest() which may emit the "complete" event (if the response is already completed) while another thread T2 (the sender thread) may still be busy sending the request content from HttpSender.ContentSender, accessing the request's ContentSource to read and demand.

This may be a problem with reproducible request content, for example in case of redirects: the redirect will try to abort the request at the "response success" event from thread T1; the request abort immediately emits the "complete" event, which triggers the call to the request's Content.Source.rewind(), but there is still thread T2 reading/demand from the request ContentSource from the HttpSender.ContentSender.

The request abort should be serialized, so that first the HttpSender.ContentSender returns from process(), then the "complete" event is triggered, and any code relying on the "complete" event is now guaranteed that there is no concurrent access to the request Content.Source.
@sbordet sbordet added the Bug For general bugs on Jetty side label Dec 11, 2023
@sbordet sbordet linked a pull request Dec 11, 2023 that will close this issue
Fixes #11037 - Serialize HttpClient request failures #11038
Merged
@sbordet sbordet moved this to 🏗 In progress in Jetty 12.0.5 - FROZEN Dec 11, 2023
@sbordet sbordet self-assigned this Dec 14, 2023
sbordet added a commit that referenced this issue Dec 15, 2023
@sbordet
Fixes #11037 - Serialize HttpClient request failures (#11038)
3167e0c 
* Introduced IteratingCallback.abort(Throwable) to serialize calls to onCompleteFailure().
* HttpSender now uses IC.abort() to serialize the abort of the request.
* Fixed HTTP/2 and HTTP/3 to dispatch the sending of the request to another thread, to free the reader thread that read the server preface.
* HTTP/1.1 does not need this, because just created connections do not need to read.
* Improved handling of request abort, as it can be from two sides: external and internal.
* Calling abort() instead of failed() for WebSocket flushers.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet
Copy link
Contributor Author

sbordet commented Dec 15, 2023

Fixed by #11038.

@sbordet sbordet closed this as completed Dec 15, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.5 - FROZEN Dec 15, 2023
@joakime joakime changed the title Jetty 12: serialize HttpClient request failures Serialize HttpClient request failures Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sbordet sbordet

Labels
Bug For general bugs on Jetty side
Projects
No open projects
Jetty 12.0.5 - FROZEN
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #11037 - Serialize HttpClient request failures jetty/jetty.project
1 participant
@sbordet