This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new daa770d0f5 Align flushing buffered writes with HTTP.
daa770d0f5 is described below
commit daa770d0f5837877ed21035625f9cbcb8822e33b
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 | 43 ++++++++++++----------------
webapps/docs/changelog.xml | 4 +++
2 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java
b/java/org/apache/coyote/ajp/AjpProcessor.java
index 6995dfe177..5878c0bdb5 100644
--- a/java/org/apache/coyote/ajp/AjpProcessor.java
+++ b/java/org/apache/coyote/ajp/AjpProcessor.java
@@ -177,13 +177,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.
*/
@@ -262,7 +255,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);
@@ -297,12 +290,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;
}
}
@@ -651,7 +648,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);
}
@@ -696,7 +694,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 {
@@ -815,9 +814,10 @@ public class AjpProcessor extends AbstractProcessor {
case Constants.SC_A_JVM_ROUTE ->
requestHeaderMessage.getBytes(tmpMB);
// nothing
- case Constants.SC_A_SSL_CERT ->
+ case Constants.SC_A_SSL_CERT -> {
// SSL certificate extraction is lazy, moved to
JkCoyoteHandler
requestHeaderMessage.getBytes(certificates);
+ }
case Constants.SC_A_SSL_CIPHER -> {
requestHeaderMessage.getBytes(tmpMB);
request.setAttribute(SSLSupport.CIPHER_SUITE_KEY,
tmpMB.toString());
@@ -826,8 +826,9 @@ public class AjpProcessor extends AbstractProcessor {
requestHeaderMessage.getBytes(tmpMB);
request.setAttribute(SSLSupport.SESSION_ID_KEY,
tmpMB.toString());
}
- case Constants.SC_A_SSL_KEY_SIZE ->
+ case Constants.SC_A_SSL_KEY_SIZE -> {
request.setAttribute(SSLSupport.KEY_SIZE_KEY,
Integer.valueOf(requestHeaderMessage.getInt()));
+ }
case Constants.SC_A_STORED_METHOD -> {
requestHeaderMessage.getBytes(tmpMB);
ByteChunk tmpBC = tmpMB.getByteChunk();
@@ -947,7 +948,6 @@ public class AjpProcessor extends AbstractProcessor {
}
tmpMB.recycle();
- responseMsgPos = -1;
int numHeaders = headers.size();
boolean needAjpMessageHeader = true;
@@ -1203,7 +1203,7 @@ public class AjpProcessor extends AbstractProcessor {
@Override
protected final boolean isReadyForWrite() {
- return responseMsgPos == -1 && socketWrapper.isReadyForWrite();
+ return socketWrapper.isReadyForWrite();
}
@@ -1276,11 +1276,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 5d574dac08..ac0fabc478 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]
