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

Also manage graceful shutdown of already started async requests #5173

Closed
wants to merge 1 commit into from
Closed

Also manage graceful shutdown of already started async requests #5173

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java
View file
Open in desktop
Expand Up @@ -189,7 +189,7 @@ else if (_wrapWarning.compareAndSet(false, true))
_dispatchedStats.decrement();
_dispatchedTimeStats.record(dispatched);

if (state.isSuspended())
if (state.isSuspended() || state.isAsyncStarted())
Copy link
Contributor

Choose a reason for hiding this comment

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

@gregw Do you think we still need to keep the state.isSuspended() check here. The state.isAsyncStarted() doesn't check for the WAITING state.

Copy link
Contributor

Choose a reason for hiding this comment

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

@thomasdraebing firstly, yes this is indeed the bug. We spotted it yesterday and were in process of fixing.

The isSuspended() check is not necessary as isAsyncStarted in his context will always be for a request that is being handled, hence all we need is the !blocking check that it will do.

@lachlan-roberts are you working on the unit tests for this plus the other things that we wanted changed? If so, then perhaps pull @thomasdraebing commit into your branch then we can close this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I rebased my changes ontop of this branch and opened PR #5175.

{
if (state.isInitial())
{
Expand Down