You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/09/29 17:47:41 UTC
(tomcat) branch 9.0.x updated: Avoid very rare race between async write notification and completion
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new ab60dfaaef Avoid very rare race between async write notification and completion
ab60dfaaef is described below
commit ab60dfaaefa0387621672db9ad1d06c3326be0e4
Author: remm <re...@apache.org>
AuthorDate: Tue Sep 29 15:12:52 2026 +0200
Avoid very rare race between async write notification and completion
Skip the write notification in these cases since it would be useless.
Some variants of TestAsync could trigger it "under load" (when running
the testsuite with multiple threads).
---
java/org/apache/coyote/AbstractProcessor.java | 11 ++++++++-
java/org/apache/coyote/AsyncStateMachine.java | 31 ++++++++++++++++++++++++++
java/org/apache/coyote/LocalStrings.properties | 1 +
webapps/docs/changelog.xml | 5 +++++
4 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/coyote/AbstractProcessor.java b/java/org/apache/coyote/AbstractProcessor.java
index c3f5dbfeff..6c319145f2 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -228,7 +228,16 @@ public abstract class AbstractProcessor extends AbstractProcessorLight implement
public final SocketState dispatch(SocketEvent status) throws IOException {
if (status == SocketEvent.OPEN_WRITE && response.getWriteListener() != null) {
- asyncStateMachine.asyncOperation();
+ if (!asyncStateMachine.asyncOperationForWriteNotification()) {
+ // 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.lateWriteNotification",
+ request.requestURI()));
+ }
+ return SocketState.LONG;
+ }
try {
if (flushBufferedWrite()) {
return SocketState.LONG;
diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java
index 56b9f30114..851bc16de9 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -264,6 +264,37 @@ class AsyncStateMachine {
}
}
+ /*
+ * 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().
+ *
+ * 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.
+ */
+ 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 ||
+ state == AsyncState.COMPLETING || state == AsyncState.MUST_DISPATCH ||
+ state == AsyncState.DISPATCH_PENDING || state == AsyncState.DISPATCHING) {
+ return false;
+ }
+ asyncOperation();
+ return true;
+ }
+
/*
* Async has been processed. Whether or not to enter a long poll depends on current state. For example, as per
* SRV.2.3.3.3 can now process calls to complete() or dispatch().
diff --git a/java/org/apache/coyote/LocalStrings.properties b/java/org/apache/coyote/LocalStrings.properties
index 597cc08dd2..1d80a2f0a1 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -33,6 +33,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.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.pushrequest.notsupported=Server push requests are not supported by this protocol
abstractProcessor.setErrorState=Error state [{0}] reported while processing request
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2eb3f14649..df69dbfcfd 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -4294,6 +4294,11 @@
meant that the first header in the response was also dropped. Fix based
on pull request <pr>710</pr> by foremans. (markt)
</fix>
+ <fix>
+ Avoid very rare race between async write notification and completion.
+ Skip the write notification in these cases since it would be useless.
+ (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org