This is an automated email from the ASF dual-hosted git repository.

markt-asf 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 4af366e210 Align flushing buffered writes with HTTP.
4af366e210 is described below

commit 4af366e21038dc76a1c2e90fff18c47c758e98a2
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Sep 29 16:50:08 2026 +0100

    Align flushing buffered writes with HTTP.
    
    - removes the responseMsgPos field as it was unused. Large non-blocking
      writes are written fully to one or more AJP messages and then
      buffered at socketWrapper as required
    - the write registration at the response layer should not be updated
      while the socket wrapper buffer is being emptied
    - reformat the code
---
 java/org/apache/coyote/ajp/AjpProcessor.java | 37 +++++++++++-----------------
 webapps/docs/changelog.xml                   |  4 +++
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java 
b/java/org/apache/coyote/ajp/AjpProcessor.java
index 499171de6b..a4813cad6c 100644
--- a/java/org/apache/coyote/ajp/AjpProcessor.java
+++ b/java/org/apache/coyote/ajp/AjpProcessor.java
@@ -182,13 +182,6 @@ public class AjpProcessor extends AbstractProcessor {
     private final AjpMessage responseMessage;
 
 
-    /**
-     * Intended to hold the location of the next write of the response message 
when non-blocking writes do not write
-     * the message in a single write. Always -1 in the current implementation 
as the write path does not update it.
-     */
-    private int responseMsgPos = -1;
-
-
     /**
      * Body message.
      */
@@ -267,7 +260,7 @@ public class AjpProcessor extends AbstractProcessor {
      * Constructs a new AjpProcessor.
      *
      * @param protocol The AJP protocol
-     * @param adapter The adapter for this processor
+     * @param adapter  The adapter for this processor
      */
     public AjpProcessor(AbstractAjpProtocol<?> protocol, Adapter adapter) {
         super(adapter);
@@ -302,12 +295,16 @@ public class AjpProcessor extends AbstractProcessor {
 
     @Override
     protected boolean flushBufferedWrite() throws IOException {
-        if (hasDataToWrite()) {
+        if (socketWrapper.hasDataToWrite()) {
             socketWrapper.flush(false);
-            if (hasDataToWrite()) {
-                // There is data to write but go via Response to
-                // maintain a consistent view of non-blocking state
-                response.checkRegisterForWrite();
+            if (socketWrapper.hasDataToWrite()) {
+                /*
+                 * The socketWrapper wasn't fully flushed so re-register the 
socket for write. Note this does not go via
+                 * the Response since the write registration state at that 
level should remain unchanged. Once the
+                 * socketWrapper has been emptied then the registration below 
will trigger a call to
+                 * Adaptor.asyncDispatch() which will enable the Response to 
respond to this event.
+                 */
+                socketWrapper.registerWriteInterest();
                 return true;
             }
         }
@@ -655,7 +652,8 @@ public class AjpProcessor extends AbstractProcessor {
         if (methodCode != Constants.SC_M_JK_STORED) {
             String methodName = Constants.getMethodForCode(methodCode - 1);
             if (methodName == null) {
-                throw new 
IllegalArgumentException(sm.getString("ajpprocessor.request.invalidMethod", 
String.valueOf(methodCode)));
+                throw new IllegalArgumentException(
+                        sm.getString("ajpprocessor.request.invalidMethod", 
String.valueOf(methodCode)));
             }
             request.setMethod(methodName);
         }
@@ -700,7 +698,8 @@ public class AjpProcessor extends AbstractProcessor {
                 requestHeaderMessage.getInt(); // To advance the read position
                 hName = Constants.getHeaderForCode(hId - 1);
                 if (hName == null) {
-                    throw new 
IllegalArgumentException(sm.getString("ajpprocessor.request.invalidHeader", 
String.valueOf(hId)));
+                    throw new IllegalArgumentException(
+                            sm.getString("ajpprocessor.request.invalidHeader", 
String.valueOf(hId)));
                 }
                 vMB = headers.addValue(hName);
             } else {
@@ -970,7 +969,6 @@ public class AjpProcessor extends AbstractProcessor {
         }
 
         tmpMB.recycle();
-        responseMsgPos = -1;
 
         int numHeaders = headers.size();
         boolean needAjpMessageHeader = true;
@@ -1226,7 +1224,7 @@ public class AjpProcessor extends AbstractProcessor {
 
     @Override
     protected final boolean isReadyForWrite() {
-        return responseMsgPos == -1 && socketWrapper.isReadyForWrite();
+        return socketWrapper.isReadyForWrite();
     }
 
 
@@ -1299,11 +1297,6 @@ public class AjpProcessor extends AbstractProcessor {
     }
 
 
-    private boolean hasDataToWrite() {
-        return responseMsgPos != -1 || socketWrapper.hasDataToWrite();
-    }
-
-
     @Override
     protected Log getLog() {
         return log;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index f34aa0f575..426cfefeb6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -154,6 +154,10 @@
         <pr>1073</pr>: Fix possible corruption when using HTTP/2 and async IO
         on uploads. Submitted by Tim Burke. (remm)
       </fix>
+      <fix>
+        Fix a timing issue that could cause large non-blocking writes to stall
+        for AJP connections. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Other">


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to