You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2020/02/12 17:36:30 UTC
[tomcat] branch master updated: Fix timeout on zero length
read/write with useAsyncIO for NIO and APR
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 5a36c39 Fix timeout on zero length read/write with useAsyncIO for NIO and APR
5a36c39 is described below
commit 5a36c3941e99d7e1554bc68cd2acef53c76d5fcb
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Feb 12 17:27:25 2020 +0000
Fix timeout on zero length read/write with useAsyncIO for NIO and APR
---
java/org/apache/tomcat/util/net/AprEndpoint.java | 1 +
java/org/apache/tomcat/util/net/Nio2Endpoint.java | 9 ---------
java/org/apache/tomcat/util/net/NioEndpoint.java | 4 ++--
java/org/apache/tomcat/util/net/SocketWrapperBase.java | 9 +++++++++
webapps/docs/changelog.xml | 10 ++++++++++
5 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java
index b8b71e3..ec9deee 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -2646,6 +2646,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB
}
if (buffer == null && flushBytes == 0) {
// Nothing to do
+ completion.completed(Long.valueOf(0), this);
return;
}
if (read) {
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 35c9df8..2175667 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -727,15 +727,6 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
}
- private static boolean arrayHasData(ByteBuffer[] byteBuffers) {
- for (ByteBuffer byteBuffer : byteBuffers) {
- if (byteBuffer.hasRemaining()) {
- return true;
- }
- }
- return false;
- }
-
public void setSendfileData(SendfileData sf) { this.sendfileData = sf; }
public SendfileData getSendfileData() { return this.sendfileData; }
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index d111b7c..773b4d6 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1457,7 +1457,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
updateLastWrite();
}
}
- if (nBytes != 0) {
+ if (nBytes != 0 || !arrayHasData(buffers)) {
completionDone = false;
}
}
@@ -1465,7 +1465,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
setError(e);
}
}
- if (nBytes > 0) {
+ if (nBytes > 0 || (nBytes == 0 && !arrayHasData(buffers))) {
// The bytes processed are only updated in the completion handler
completion.completed(Long.valueOf(nBytes), this);
} else if (nBytes < 0 || getError() != null) {
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index 006e9c2..40ccd5d 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -1444,4 +1444,13 @@ public abstract class SocketWrapperBase<E> {
}
return max;
}
+
+ protected static boolean arrayHasData(ByteBuffer[] byteBuffers) {
+ for (ByteBuffer byteBuffer : byteBuffers) {
+ if (byteBuffer.hasRemaining()) {
+ return true;
+ }
+ }
+ return false;
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b844626..c3fc696 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -66,6 +66,16 @@
</fix>
</changelog>
</subsection>
+ <subsection name="Coyote">
+ <changelog>
+ <fix>
+ When the NIO or APR/native connectors were configured with
+ <code>useAsyncIO="true"</code> and a zero length read or write was
+ performed, the read/write would time out rather than return immediately.
+ (markt)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Jasper">
<changelog>
<scode>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch master updated: Fix timeout on zero length
read/write with useAsyncIO for NIO and APR
Posted by Rémy Maucherat <re...@apache.org>.
On Wed, Feb 12, 2020 at 8:02 PM Mark Thomas <ma...@apache.org> wrote:
> On 12/02/2020 18:53, Rémy Maucherat wrote:
>
> <snip/>
>
> > I think I remember I had problems already before the async API with
> > those weird tests doing zero length writes in that TCK :(
> >
> > I may make changes to the fix, since:
> > - I notice arrayHasData doesn't use offset and length, it's better if it
> > does
> > - With NIO arrayHasData is also used for a read, I suppose it's not a
> > problem but I'll change it anyway
>
> Thanks for the review.
>
> For read, won't arrayHasData() check if there is any buffer space to
> write into? Might need to change the method name in that case. maybe
> arrayRemaining() ?
>
So that looks ok. I'll add use of the offset/length shortly, it won't hurt.
Rémy
Re: [tomcat] branch master updated: Fix timeout on zero length
read/write with useAsyncIO for NIO and APR
Posted by Mark Thomas <ma...@apache.org>.
On 12/02/2020 18:53, Rémy Maucherat wrote:
<snip/>
> I think I remember I had problems already before the async API with
> those weird tests doing zero length writes in that TCK :(
>
> I may make changes to the fix, since:
> - I notice arrayHasData doesn't use offset and length, it's better if it
> does
> - With NIO arrayHasData is also used for a read, I suppose it's not a
> problem but I'll change it anyway
Thanks for the review.
For read, won't arrayHasData() check if there is any buffer space to
write into? Might need to change the method name in that case. maybe
arrayRemaining() ?
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch master updated: Fix timeout on zero length
read/write with useAsyncIO for NIO and APR
Posted by Rémy Maucherat <re...@apache.org>.
On Wed, Feb 12, 2020 at 6:42 PM Mark Thomas <ma...@apache.org> wrote:
> On 12/02/2020 17:36, markt@apache.org wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > markt pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> > new 5a36c39 Fix timeout on zero length read/write with useAsyncIO
> for NIO and APR
> > 5a36c39 is described below
> >
> > commit 5a36c3941e99d7e1554bc68cd2acef53c76d5fcb
> > Author: Mark Thomas <ma...@apache.org>
> > AuthorDate: Wed Feb 12 17:27:25 2020 +0000
> >
> > Fix timeout on zero length read/write with useAsyncIO for NIO and APR
>
> For those wondering about this, I found this while testing the Jakarta
> WebSocket 2.0 TCK with Tomcat 10. A couple of the tests switch from
> buffered to non-buffered messages with any empty buffer and the
> subsequent flush triggers a zero length write. Whether that write should
> even be triggered is a whole other question...
>
I think I remember I had problems already before the async API with those
weird tests doing zero length writes in that TCK :(
I may make changes to the fix, since:
- I notice arrayHasData doesn't use offset and length, it's better if it
does
- With NIO arrayHasData is also used for a read, I suppose it's not a
problem but I'll change it anyway
Rémy
Re: [tomcat] branch master updated: Fix timeout on zero length
read/write with useAsyncIO for NIO and APR
Posted by Mark Thomas <ma...@apache.org>.
On 12/02/2020 17:36, markt@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new 5a36c39 Fix timeout on zero length read/write with useAsyncIO for NIO and APR
> 5a36c39 is described below
>
> commit 5a36c3941e99d7e1554bc68cd2acef53c76d5fcb
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Wed Feb 12 17:27:25 2020 +0000
>
> Fix timeout on zero length read/write with useAsyncIO for NIO and APR
For those wondering about this, I found this while testing the Jakarta
WebSocket 2.0 TCK with Tomcat 10. A couple of the tests switch from
buffered to non-buffered messages with any empty buffer and the
subsequent flush triggers a zero length write. Whether that write should
even be triggered is a whole other question...
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org