This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 85daf45655 Follow-up to 09c0caced9
85daf45655 is described below
commit 85daf4565576e76367978ad8bb27a57d15a27fd4
Author: Mark Thomas <[email protected]>
AuthorDate: Wed Sep 30 10:05:46 2026 +0100
Follow-up to 09c0caced9
Changes
- STARTING or READ_WRITE_OP not possible after complete()/dispatch()
- Handle the same race for read
---
java/org/apache/coyote/AbstractProcessor.java | 19 +++++++---
java/org/apache/coyote/AsyncStateMachine.java | 50 ++++++++++----------------
java/org/apache/coyote/LocalStrings.properties | 1 +
java/org/apache/coyote/ajp/AjpProcessor.java | 5 +--
4 files changed, 37 insertions(+), 38 deletions(-)
diff --git a/java/org/apache/coyote/AbstractProcessor.java
b/java/org/apache/coyote/AbstractProcessor.java
index c5667b93af..3b8b9cb73a 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -228,7 +228,7 @@ public abstract class AbstractProcessor extends
AbstractProcessorLight implement
public final SocketState dispatch(SocketEvent status) throws IOException {
if (status == SocketEvent.OPEN_WRITE && response.getWriteListener() !=
null) {
- if (!asyncStateMachine.asyncOperationForWriteNotification()) {
+ if (!asyncStateMachine.asyncOperation()) {
// The notification raced with async completion on another
// thread. Nothing to notify; returning LONG lets
// asyncPostProcess() complete the cycle as usual.
@@ -250,7 +250,16 @@ public abstract class AbstractProcessor extends
AbstractProcessorLight implement
request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, ioe);
}
} else if (status == SocketEvent.OPEN_READ &&
request.getReadListener() != null) {
- dispatchNonBlockingRead();
+ if (!dispatchNonBlockingRead()) {
+ // The notification raced with async completion on another
+ // thread. Nothing to notify; returning LONG lets
+ // asyncPostProcess() complete the cycle as usual.
+ if (getLog().isTraceEnabled()) {
+
getLog().trace(sm.getString("abstractProcessor.lateReadNotification",
+ request.requestURI()));
+ }
+ return SocketState.LONG;
+ }
} else if (status == SocketEvent.ERROR) {
// An I/O error occurred on a non-container thread. This includes:
// - read/write timeouts fired by the Poller in NIO
@@ -709,9 +718,11 @@ public abstract class AbstractProcessor extends
AbstractProcessorLight implement
/**
* Perform any necessary processing for a non-blocking read before
dispatching to the adapter.
+ *
+ * @return {@code true} if the read listener should be notified, otherwise
{@code false}
*/
- protected void dispatchNonBlockingRead() {
- asyncStateMachine.asyncOperation();
+ protected boolean dispatchNonBlockingRead() {
+ return asyncStateMachine.asyncOperation();
}
diff --git a/java/org/apache/coyote/AsyncStateMachine.java
b/java/org/apache/coyote/AsyncStateMachine.java
index 129b7bf88f..858b19bcf3 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -252,44 +252,30 @@ class AsyncStateMachine {
}
}
- synchronized void asyncOperation() {
- if (state == AsyncState.STARTED) {
- updateState(AsyncState.READ_WRITE_OP);
- } else {
- throw new IllegalStateException(
- sm.getString("asyncStateMachine.invalidAsyncState",
"asyncOperation()", state));
- }
- }
-
- /*
- * Entry point for transport generated OPEN_WRITE write-listener
- * notifications. Unlike asyncOperation(), which the container calls when
- * it is about to perform an application initiated non-blocking write, a
- * notification may legitimately race with the completion of the async
- * cycle: the transport may queue the event while the write listener is
- * still active and only deliver the dispatch after the application has
- * completed the response on another thread. Once the cycle is completing
- * there is nothing left to notify (the listener will not be called again)
- * and the completion path in asyncPostProcess() takes care of flushing
- * buffered data and firing onComplete().
+ /**
+ * Updates the state machine after a "ready for read" or "ready for write"
notification has been received from the
+ * network layer.
*
- * Returns true if the caller should notify the write listener, false if
- * the notification raced with completion and should be treated as
- * handled without notifying the listener.
+ * @return {@code true} if the caller should notify the associated
listener, otherwise {@code false}
*/
- synchronized boolean asyncOperationForWriteNotification() {
- // States in which the cycle is completing (or an application
- // initiated write operation is already in progress): asyncPostProcess
- // () has a completion branch for each of these, so ignoring the
- // notification lets the completion proceed normally.
- if (state == AsyncState.READ_WRITE_OP || state == AsyncState.STARTING
||
- state == AsyncState.MUST_COMPLETE || state ==
AsyncState.COMPLETE_PENDING ||
+ synchronized boolean asyncOperation() {
+ if (state == AsyncState.STARTED) {
+ updateState(AsyncState.READ_WRITE_OP);
+ return true;
+ } else if (state == AsyncState.MUST_COMPLETE || state ==
AsyncState.COMPLETE_PENDING ||
state == AsyncState.COMPLETING || state ==
AsyncState.MUST_DISPATCH ||
state == AsyncState.DISPATCH_PENDING || state ==
AsyncState.DISPATCHING) {
+ /*
+ * It is possible that a read or write notification could race
with a completion or dispatch. In that
+ * scenario if the complete/dispatch win the later call to this
method triggered by the read/write
+ * notification will trigger an ISE. Therefore, if the state
machine has already processed a
+ * complete/dispatch, ignore the read/write notification.
+ */
return false;
+ } else {
+ throw new IllegalStateException(
+ sm.getString("asyncStateMachine.invalidAsyncState",
"asyncOperation()", state));
}
- asyncOperation();
- return true;
}
/*
diff --git a/java/org/apache/coyote/LocalStrings.properties
b/java/org/apache/coyote/LocalStrings.properties
index e806321ab3..385f9713fd 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -30,6 +30,7 @@ abstractProcessor.fallToDebug=\n\
\ Note: further occurrences of request parsing errors will be logged at DEBUG
level.
abstractProcessor.hostInvalid=The host [{0}] is not valid
abstractProcessor.httpupgrade.notsupported=HTTP upgrade is not supported by
this protocol
+abstractProcessor.lateReadNotification=Ignoring late read notification for
request [{0}] because the async cycle is completing
abstractProcessor.lateWriteNotification=Ignoring late write notification for
request [{0}] because the async cycle is completing
abstractProcessor.noExecute=Unable to transfer processing to a container
thread because this Processor is not currently associated with a SocketWrapper
abstractProcessor.setErrorState=Error state [{0}] reported while processing
request
diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java
b/java/org/apache/coyote/ajp/AjpProcessor.java
index 5b88662683..ba5f56e0cd 100644
--- a/java/org/apache/coyote/ajp/AjpProcessor.java
+++ b/java/org/apache/coyote/ajp/AjpProcessor.java
@@ -308,10 +308,11 @@ public class AjpProcessor extends AbstractProcessor {
@Override
- protected void dispatchNonBlockingRead() {
+ protected boolean dispatchNonBlockingRead() {
if (available(true) > 0) {
- super.dispatchNonBlockingRead();
+ return super.dispatchNonBlockingRead();
}
+ return true;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
