Skip to content

Discard remaining REST handlers when the client connection is closed prematurely #50990

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

Merged
merged 1 commit into from
Nov 13, 2025
Merged

Discard remaining REST handlers when the client connection is closed prematurely #50990

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions ...src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ public void restart(RuntimeResource target, boolean setLocatorTarget) {
public void setupInitialMatchAndRestart(RequestMapper.RequestMatch<RestInitialHandler.InitialMatch> initialMatch) {
this.initialMatch = initialMatch;

// add a default close handler that simply discards whatever REST handlers still remain to be run
serverResponse().addCloseHandler(new DiscardRemainingRunner(this));

restart(initialMatch.value.handlers);
setMaxPathParams(initialMatch.value.maxPathParams);
setRemaining(initialMatch.remaining);
Expand Down Expand Up @@ -435,6 +438,14 @@ public void close() {
super.close();
}

/**
* This method ensures that no more handlers will run and that all the resources tied to the request are closed
*/
private void discardRemaining() {
setPosition(getHandlers().length);
close();
}

public LazyResponse getResponse() {
return response;
}
Expand Down Expand Up @@ -1330,4 +1341,24 @@ public PreviousResource(RuntimeResource locatorTarget, Object locatorPathParamVa
private final PreviousResource prev;

}

private static class DiscardRemainingRunner implements Runnable {

private ResteasyReactiveRequestContext context;

private DiscardRemainingRunner(ResteasyReactiveRequestContext context) {
this.context = context;
}

@Override
public void run() {
try {
context.discardRemaining();
Comment on lines +1355 to +1356
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

There's a potential race condition: the close handler could be invoked from another thread while the request is still being processed. If discardRemaining() is called while handlers are executing, setPosition(getHandlers().length) could interfere with the handler chain execution. Consider adding synchronization or checking if the request context is already closed before modifying the position.

Copilot uses AI. Check for mistakes.
Copy link
Member

@FroMage FroMage Nov 13, 2025

Choose a reason for hiding this comment

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

Heh, I was going to say exactly the same thing. I'm impressed.

I'm not sure what thread invokes the close handler (I guess/hope the same request's context/thread) and what other thread might be running the request (especially in the case of blocking requests).

So this might indeed have more than one thread interact with the handler position in parallel. Needs to be checked.

} catch (Exception ignored) {

Comment on lines +1355 to +1358
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Catching and silently ignoring all exceptions without any logging could hide important errors. Consider at least logging the exception at debug or trace level to aid in troubleshooting issues related to connection closures and handler cleanup.

Copilot uses AI. Check for mistakes.
} finally {
context = null;
}
}
}
}
Loading