Author: remm
Date: Thu Feb 21 16:37:20 2019
New Revision: 1854066
URL: http://svn.apache.org/viewvc?rev=1854066&view=rev
Log:
Refactor to redo fix for 63182. The root cause is that the pending flag is
released once processing start, and concurrent unsynced access from non
container threads can cause awaitBytes to happen concurrently.
Modified:
tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java
tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java
tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java
tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Thu Feb 21
16:37:20 2019
@@ -790,7 +790,7 @@ public abstract class AbstractProcessor
}
if (!isRequestBodyFullyRead()) {
- registerReadInterest();
+ registerReadInterest(true);
}
return false;
@@ -800,7 +800,7 @@ public abstract class AbstractProcessor
protected abstract boolean isRequestBodyFullyRead();
- protected abstract void registerReadInterest();
+ protected abstract void registerReadInterest(boolean body);
protected abstract boolean isReadyForWrite();
Modified: tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java (original)
+++ tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java Thu Feb 21
16:37:20 2019
@@ -903,7 +903,7 @@ public abstract class AbstractProtocol<S
// processor. Continue to poll for the next request.
connections.remove(socket);
release(processor);
- wrapper.registerReadInterest();
+ wrapper.registerReadInterest(true);
} else if (state == SocketState.SENDFILE) {
// Sendfile in progress. If it fails, the socket will be
// closed. If it works, the socket either be added to the
@@ -993,7 +993,7 @@ public abstract class AbstractProtocol<S
// - this is an upgraded connection
// - the request line/headers have not been completely
// read
- socket.registerReadInterest();
+ socket.registerReadInterest(true);
}
}
Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Thu Feb 21
16:37:20 2019
@@ -1134,8 +1134,8 @@ public class AjpProcessor extends Abstra
@Override
- protected final void registerReadInterest() {
- socketWrapper.registerReadInterest();
+ protected final void registerReadInterest(boolean body) {
+ socketWrapper.registerReadInterest(!body);
}
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Thu Feb 21
16:37:20 2019
@@ -1215,8 +1215,8 @@ public class Http11Processor extends Abs
@Override
- protected final void registerReadInterest() {
- socketWrapper.registerReadInterest();
+ protected final void registerReadInterest(boolean body) {
+ socketWrapper.registerReadInterest(!body);
}
Modified:
tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
---
tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java
(original)
+++
tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java
Thu Feb 21 16:37:20 2019
@@ -107,7 +107,7 @@ public class UpgradeServletInputStream e
if (ContainerThreadMarker.isContainerThread()) {
processor.addDispatch(DispatchType.NON_BLOCKING_READ);
} else {
- socketWrapper.registerReadInterest();
+ socketWrapper.registerReadInterest(true);
}
// Switching to non-blocking. Don't know if data is available.
Modified: tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java Thu Feb 21
16:37:20 2019
@@ -263,7 +263,7 @@ class StreamProcessor extends AbstractPr
@Override
- protected final void registerReadInterest() {
+ protected final void registerReadInterest(boolean body) {
// Should never be called for StreamProcessor as isReadyForRead() is
// overridden
throw new UnsupportedOperationException();
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Thu Feb 21
16:37:20 2019
@@ -2724,7 +2724,7 @@ public class AprEndpoint extends Abstrac
@Override
- public void registerReadInterest() {
+ public void registerReadInterest(boolean polling) {
// Make sure an already closed socket is not added to the poller
synchronized (closedLock) {
if (closed) {
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Thu Feb 21
16:37:20 2019
@@ -590,7 +590,7 @@ public class Nio2Endpoint extends Abstra
this.readCompletionHandler = new CompletionHandler<Integer,
ByteBuffer>() {
@Override
public void completed(Integer nBytes, ByteBuffer attachment) {
- boolean notify = false;
+ boolean readNotify = false;
if (log.isDebugEnabled()) {
log.debug("Socket: [" + Nio2SocketWrapper.this + "],
Interest: [" + readInterest + "]");
}
@@ -599,16 +599,16 @@ public class Nio2Endpoint extends Abstra
failed(new EOFException(), attachment);
} else {
if (readInterest && !Nio2Endpoint.isInline()) {
- readInterest = false;
- notify = true;
+ readNotify = true;
} else {
// Release here since there will be no
// notify/dispatch to do the release.
readPending.release();
}
+ readInterest = false;
}
}
- if (notify) {
+ if (readNotify) {
getEndpoint().processSocket(Nio2SocketWrapper.this,
SocketEvent.OPEN_READ, false);
}
}
@@ -810,7 +810,7 @@ public class Nio2Endpoint extends Abstra
socketBufferHandler.configureReadBufferForRead();
nRead = Math.min(nRead, len);
socketBufferHandler.getReadBuffer().get(b, off, nRead);
- } else if (nRead == 0 && !block &&
ContainerThreadMarker.isContainerThread()) {
+ } else if (nRead == 0 && !block) {
readInterest = true;
}
if (log.isDebugEnabled()) {
@@ -873,7 +873,7 @@ public class Nio2Endpoint extends Abstra
// data that was just read
if (nRead > 0) {
nRead = populateReadBuffer(to);
- } else if (nRead == 0 && !block &&
ContainerThreadMarker.isContainerThread()) {
+ } else if (nRead == 0 && !block) {
readInterest = true;
}
}
@@ -1475,11 +1475,11 @@ public class Nio2Endpoint extends Abstra
@Override
- public void registerReadInterest() {
+ public void registerReadInterest(boolean polling) {
synchronized (readCompletionHandler) {
if (readPending.availablePermits() == 0) {
readInterest = true;
- } else {
+ } else if (polling) {
// If no read is pending, start waiting for data
awaitBytes();
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Thu Feb 21
16:37:20 2019
@@ -1243,7 +1243,7 @@ public class NioEndpoint extends Abstrac
@Override
- public void registerReadInterest() {
+ public void registerReadInterest(boolean polling) {
getPoller().add(getSocket(), SelectionKey.OP_READ);
}
@@ -1420,7 +1420,7 @@ public class NioEndpoint extends Abstrac
} else if (handshake == -1 ) {
close(socket, key);
} else if (handshake == SelectionKey.OP_READ){
- socketWrapper.registerReadInterest();
+ socketWrapper.registerReadInterest(true);
} else if (handshake == SelectionKey.OP_WRITE){
socketWrapper.registerWriteInterest();
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java Thu Feb
21 16:37:20 2019
@@ -763,7 +763,7 @@ public abstract class SocketWrapperBase<
}
- public abstract void registerReadInterest();
+ public abstract void registerReadInterest(boolean polling);
public abstract void registerWriteInterest();
Modified:
tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java?rev=1854066&r1=1854065&r2=1854066&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
Thu Feb 21 16:37:20 2019
@@ -124,6 +124,9 @@ public class TestNonBlockingAPI extends
Assert.assertEquals(HttpServletResponse.SC_OK, rc);
if (async) {
Assert.assertEquals(2000000 * 8, servlet.listener.body.length());
+ TestAsyncReadListener listener = (TestAsyncReadListener)
servlet.listener;
+ Assert.assertEquals(listener.notReadyCount,
listener.containerThreadCount);
+ Assert.assertEquals(listener.isReadyCount,
listener.nonContainerThreadCount);
} else {
Assert.assertEquals(5 * 8, servlet.listener.body.length());
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
