Re: [tomcat] branch master updated: Fix timeout handling. Write timeout could be handled as read timeout.

2019-11-18 Thread Mark Thomas
On 17/11/2019 22:33, Rémy Maucherat wrote:
> On Sat, Nov 16, 2019 at 5:44 PM  > 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 40ca9be  Fix timeout handling. Write timeout could be
> handled as read timeout.
> 40ca9be is described below
> 
> commit 40ca9bef10a2c2db48e759b16fc4fbff3725ffca
> Author: Mark Thomas mailto:ma...@apache.org>>
> AuthorDate: Sat Nov 16 16:43:52 2019 +
> 
>     Fix timeout handling. Write timeout could be handled as read
> timeout.



> I should have figured out that I needed only two flags there. Sorry for
> the trouble, it must have been a lot harder to figure out a long time
> after the initial overhaul.

No trouble. I'm not even sure that this was causing a problem. I ended
up looking at the code while investigating an intermittent test failure.
The error appeared to go away after I made the change but these sorts of
failures being what they are it may have been completely unrelated to
this change. I guess time will tell.

Mark

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



Re: [tomcat] branch master updated: Fix timeout handling. Write timeout could be handled as read timeout.

2019-11-17 Thread Rémy Maucherat
On Sat, Nov 16, 2019 at 5:44 PM  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 40ca9be  Fix timeout handling. Write timeout could be handled as
> read timeout.
> 40ca9be is described below
>
> commit 40ca9bef10a2c2db48e759b16fc4fbff3725ffca
> Author: Mark Thomas 
> AuthorDate: Sat Nov 16 16:43:52 2019 +
>
> Fix timeout handling. Write timeout could be handled as read timeout.
> ---
>  java/org/apache/tomcat/util/net/NioEndpoint.java | 15 ---
>  webapps/docs/changelog.xml   |  4 
>  2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
> b/java/org/apache/tomcat/util/net/NioEndpoint.java
> index 1e5b900..090b26a 100644
> --- a/java/org/apache/tomcat/util/net/NioEndpoint.java
> +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
> @@ -953,24 +953,25 @@ public class NioEndpoint extends
> AbstractJsseEndpoint
>  cancelledKey(key, socketWrapper);
>  } else if ((socketWrapper.interestOps() &
> SelectionKey.OP_READ) == SelectionKey.OP_READ ||
>(socketWrapper.interestOps() &
> SelectionKey.OP_WRITE) == SelectionKey.OP_WRITE) {
> -boolean isTimedOut = false;
>  boolean readTimeout = false;
>  boolean writeTimeout = false;
>

I should have figured out that I needed only two flags there. Sorry for the
trouble, it must have been a lot harder to figure out a long time after the
initial overhaul.

Rémy


[tomcat] branch master updated: Fix timeout handling. Write timeout could be handled as read timeout.

2019-11-16 Thread markt
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 40ca9be  Fix timeout handling. Write timeout could be handled as read 
timeout.
40ca9be is described below

commit 40ca9bef10a2c2db48e759b16fc4fbff3725ffca
Author: Mark Thomas 
AuthorDate: Sat Nov 16 16:43:52 2019 +

Fix timeout handling. Write timeout could be handled as read timeout.
---
 java/org/apache/tomcat/util/net/NioEndpoint.java | 15 ---
 webapps/docs/changelog.xml   |  4 
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java 
b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 1e5b900..090b26a 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -953,24 +953,25 @@ public class NioEndpoint extends 
AbstractJsseEndpoint
 cancelledKey(key, socketWrapper);
 } else if ((socketWrapper.interestOps() & 
SelectionKey.OP_READ) == SelectionKey.OP_READ ||
   (socketWrapper.interestOps() & 
SelectionKey.OP_WRITE) == SelectionKey.OP_WRITE) {
-boolean isTimedOut = false;
 boolean readTimeout = false;
 boolean writeTimeout = false;
 // Check for read timeout
 if ((socketWrapper.interestOps() & 
SelectionKey.OP_READ) == SelectionKey.OP_READ) {
 long delta = now - socketWrapper.getLastRead();
 long timeout = socketWrapper.getReadTimeout();
-isTimedOut = timeout > 0 && delta > timeout;
-readTimeout = true;
+if (timeout > 0 && delta > timeout) {
+readTimeout = true;
+}
 }
 // Check for write timeout
-if (!isTimedOut && (socketWrapper.interestOps() & 
SelectionKey.OP_WRITE) == SelectionKey.OP_WRITE) {
+if (!readTimeout && (socketWrapper.interestOps() & 
SelectionKey.OP_WRITE) == SelectionKey.OP_WRITE) {
 long delta = now - 
socketWrapper.getLastWrite();
 long timeout = socketWrapper.getWriteTimeout();
-isTimedOut = timeout > 0 && delta > timeout;
-writeTimeout = true;
+if (timeout > 0 && delta > timeout) {
+writeTimeout = true;
+}
 }
-if (isTimedOut) {
+if (readTimeout || writeTimeout) {
 key.interestOps(0);
 // Avoid duplicate timeout calls
 socketWrapper.interestOps(0);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c209d6e..c60f634 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -57,6 +57,10 @@
   
 63835: Add support for Keep-Alive response header. 
(michaelo)
   
+  
+Correct a logic bug in the NioEndpoint timeout handling
+that meant a write timeout could be handled as a read timeout. (markt)
+  
 
   
   


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org