On Wed, Sep 30, 2026 at 11:06 AM <[email protected]> wrote:
>
> 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
My test endpoint is a bit overeager for write, and no other issue was
found. So probably it's not quite needed for read in practice (but you
never know I guess).
Rémy
> ---
> 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]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
