This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 38dc12b Harmonize NIO2 isReadyForWrite with isReadyForRead code
38dc12b is described below
commit 38dc12b640dec45037b000c9af48738483553302
Author: remm <[email protected]>
AuthorDate: Wed Mar 27 11:35:49 2019 +0100
Harmonize NIO2 isReadyForWrite with isReadyForRead code
Following 8.5 #1717, it is better to redo it with sync as it seems to
indicate a leftover write issue.
Also add cleanups.
---
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 121 ++++++++++++++-------
.../apache/tomcat/util/net/SocketWrapperBase.java | 2 +
webapps/docs/changelog.xml | 3 +
3 files changed, 84 insertions(+), 42 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 992063f..9182aec 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -111,6 +111,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
// --------------------------------------------------------- Public Methods
+
/**
* Number of keep-alive sockets.
*
@@ -132,7 +133,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
public void bind() throws Exception {
// Create worker collection
- if ( getExecutor() == null ) {
+ if (getExecutor() == null) {
createExecutor();
}
if (getExecutor() instanceof ExecutorService) {
@@ -146,7 +147,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
serverSock = AsynchronousServerSocketChannel.open(threadGroup);
socketProperties.setProperties(serverSock);
InetSocketAddress addr = (getAddress()!=null?new
InetSocketAddress(getAddress(),getPort()):new InetSocketAddress(getPort()));
- serverSock.bind(addr,getAcceptCount());
+ serverSock.bind(addr, getAcceptCount());
// Initialize thread count defaults for acceptor, poller
if (acceptorThreadCount != 1) {
@@ -339,12 +340,12 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
protected SocketProcessorBase<Nio2Channel> createSocketProcessor(
SocketWrapperBase<Nio2Channel> socketWrapper, SocketEvent event) {
return new SocketProcessor(socketWrapper, event);
- }
+ }
@Override
protected Log getLog() {
return log;
- }
+ }
@Override
@@ -715,9 +716,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
}
int nRead = fillReadBuffer(false);
-
boolean isReady = nRead > 0;
-
if (!isReady) {
readInterest = true;
}
@@ -727,6 +726,39 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
@Override
+ public boolean isReadyForWrite() {
+ synchronized (writeCompletionHandler) {
+ if (writeNotify) {
+ return true;
+ }
+
+ if (!writePending.tryAcquire()) {
+ writeInterest = true;
+ return false;
+ }
+
+ if (socketBufferHandler.isWriteBufferEmpty() &&
nonBlockingWriteBuffer.isEmpty()) {
+ writePending.release();
+ return true;
+ }
+
+ boolean dataLeft = false;
+ try {
+ dataLeft = flushNonBlocking(true);
+ } catch (IOException e) {
+ setError(e);
+ return true;
+ }
+ boolean isReady = !dataLeft;
+ if (!isReady) {
+ writeInterest = true;
+ }
+ return isReady;
+ }
+ }
+
+
+ @Override
public int read(boolean block, byte[] b, int off, int len) throws
IOException {
checkError();
@@ -847,7 +879,6 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
readInterest = true;
}
}
-
return nRead;
}
}
@@ -903,7 +934,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
@Override
public boolean hasAsyncIO() {
- return false;
+ return true;
}
/**
@@ -1387,9 +1418,8 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
@Override
public boolean hasDataToRead() {
synchronized (readCompletionHandler) {
- return !socketBufferHandler.isReadBufferEmpty() ||
- readNotify ||
- getError() != null;
+ return !socketBufferHandler.isReadBufferEmpty()
+ || readNotify || getError() != null;
}
}
@@ -1397,8 +1427,8 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
@Override
public boolean hasDataToWrite() {
synchronized (writeCompletionHandler) {
- return !socketBufferHandler.isWriteBufferEmpty() ||
writeNotify ||
- !nonBlockingWriteBuffer.isEmpty() || getError() !=
null;
+ return !socketBufferHandler.isWriteBufferEmpty() ||
!nonBlockingWriteBuffer.isEmpty()
+ || writeNotify || writePending.availablePermits() == 0
|| getError() != null;
}
}
@@ -1412,39 +1442,46 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
@Override
- public boolean awaitReadComplete(long timeout, TimeUnit unit) {
- try {
- if (readPending.tryAcquire(timeout, unit)) {
- readPending.release();
- }
- } catch (InterruptedException e) {
- return false;
+ public boolean isWritePending() {
+ synchronized (writeCompletionHandler) {
+ return writePending.availablePermits() == 0;
}
- return true;
}
@Override
- public boolean awaitWriteComplete(long timeout, TimeUnit unit) {
- try {
- if (writePending.tryAcquire(timeout, unit)) {
- writePending.release();
+ public boolean awaitReadComplete(long timeout, TimeUnit unit) {
+ synchronized (readCompletionHandler) {
+ try {
+ if (readNotify) {
+ return true;
+ } else if (readPending.tryAcquire(timeout, unit)) {
+ readPending.release();
+ return true;
+ } else {
+ return false;
+ }
+ } catch (InterruptedException e) {
+ return false;
}
- } catch (InterruptedException e) {
- return false;
}
- return true;
}
- /*
- * This should only be called from a thread that currently holds a lock
- * on the socket. This prevents a race condition between a pending read
- * being completed and processed and a thread triggering a new read.
- */
- void releaseReadPending() {
- synchronized (readCompletionHandler) {
- if (readPending.availablePermits() == 0) {
- readPending.release();
+
+ @Override
+ public boolean awaitWriteComplete(long timeout, TimeUnit unit) {
+ synchronized (writeCompletionHandler) {
+ try {
+ if (writeNotify) {
+ return true;
+ } else if (writePending.tryAcquire(timeout, unit)) {
+ writePending.release();
+ return true;
+ } else {
+ return false;
+ }
+ } catch (InterruptedException e) {
+ return false;
}
}
}
@@ -1459,12 +1496,12 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
}
readInterest = true;
if (readPending.tryAcquire()) {
- // No read pending, so await bytes
+ // No read pending, so do a read
try {
if (fillReadBuffer(false) > 0) {
- // Special case where the read completes inline,
there is no notification
- // in that case and it cannot happen elsewhere
-
getEndpoint().processSocket(Nio2SocketWrapper.this, SocketEvent.OPEN_READ,
true);
+ // Special case where the read completed inline,
there is no notification
+ // in that case so it has to be done here
+ getEndpoint().processSocket(this,
SocketEvent.OPEN_READ, true);
}
} catch (IOException e) {
// Will never happen
@@ -1484,7 +1521,7 @@ public class Nio2Endpoint extends
AbstractJsseEndpoint<Nio2Channel> {
}
writeInterest = true;
if (writePending.availablePermits() == 1) {
- // If no write is pending, notify
+ // If no write is pending, notify that writing is possible
getEndpoint().processSocket(this, SocketEvent.OPEN_WRITE,
true);
}
}
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index df84510..c3097d6 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -953,6 +953,7 @@ public abstract class SocketWrapperBase<E> {
* <code>false</code> if the operation is still pending and
* the specified timeout has passed
*/
+ @Deprecated
public boolean awaitReadComplete(long timeout, TimeUnit unit) {
return true;
}
@@ -967,6 +968,7 @@ public abstract class SocketWrapperBase<E> {
* <code>false</code> if the operation is still pending and
* the specified timeout has passed
*/
+ @Deprecated
public boolean awaitWriteComplete(long timeout, TimeUnit unit) {
return true;
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 86ec3a7..fc04dd7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -90,6 +90,9 @@
<fix>
Refactor NIO2 write pending strategy for the classic IO API. (remm)
</fix>
+ <fix>
+ Harmonize NIO2 isReadyForWrite with isReadyForRead code. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
