-
Notifications
You must be signed in to change notification settings - Fork 34
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.
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
UNDERTOW-1701: Fix race condition in GracefulShutdownHandler #72
base: main
Are you sure you want to change the base?
Conversation
activeRequestsUpdater.incrementAndGet(this); | ||
if (shutdown) { | ||
long snapshot = stateUpdater.updateAndGet(this, incrementActive); | ||
if (isShutdown(snapshot)) { | ||
decrementRequests(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case could be optimized to avoid CAS increment+decrement after the shutdown flag has been set by extracting the LongUnaryOperator into a loop until CAS succeeds similarly to the jboss-threads view executor pattern: https://github.com/jbossas/jboss-threads/blob/a203e13fd739902f850b55a14dc9c77e46a361a6/src/main/java/org/jboss/threads/EnhancedViewExecutor.java#L250-L280
Unclear if it's helpful to optimize this case though, I'm most interested in the hot common case in which the server is actively accepting requests.
e.g.
for (;;) {
long snapshot = state;
if (isShutdown(snapshot)) {
exchange.setStatusCode(StatusCodes.SERVICE_UNAVAILABLE);
exchange.endExchange();
return;
}
if (stateUpdater.compareAndSet(this, snapshot, snapshot + 1)) {
exchange.addExchangeCompleteListener(listener);
next.handleRequest(exchange);
break;
}
}
576b26e
to
be54ee8
Compare
This comment has been minimized.
This comment has been minimized.
be54ee8
to
0b82018
Compare
Status for workflow
|
Status | Name | Step | Failures | Logs | Raw logs |
---|---|---|---|---|---|
✔️ | Build - JDK 11 | Logs | Raw logs | ||
✖ | Build - JDK 17 | Build with Maven |
Failures | Logs | Raw logs |
Full information is available in the Build summary check run.
Failures
⚙️ Build - JDK 17 #
- Failing: servlet
! Skipped: websocket websocket/core websocket/servlet and 1 more
📦 servlet
✖ io.undertow.servlet.test.dispatcher.DispatcherForwardTestCase.testPathBasedInclude
line 137
- More details - Source on GitHub
org.junit.ComparisonFailure: expected:<...etContext/dispatch /[dispatch]> but was:<...etContext/dispatch /[forward]>
at org.junit.Assert.assertEquals(Assert.java:117)
at org.junit.Assert.assertEquals(Assert.java:146)
at io.undertow.servlet.test.dispatcher.DispatcherForwardTestCase.testPathBasedInclude(DispatcherForwardTestCase.java:137)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
From undertow-io/undertow#875