Re: svn commit: r1095794 - in /tomcat/trunk: java/org/apache/coyote/http11/Http11Processor.java webapps/docs/changelog.xml

[Mark Thomas]

On 21/04/2011 20:25, Filip Hanik - Dev Lists wrote:
 On 4/21/2011 1:02 PM, ma...@apache.org wrote:
 +int firstReadTimeout;
 +if (queueTime= standardTimeout) {
 +// Queued for longer than timeout but there
 might be
 +// data so use shortest possible timeout
 +firstReadTimeout = 1;
 +} else {
 +// Cast is safe since queueTime must be less
 than
 +// standardTimeout which is an int
 +firstReadTimeout = standardTimeout - (int)
 queueTime;
 +}
 +socket.getSocket().setSoTimeout(firstReadTimeout);
 +if (!inputBuffer.fill()) {
 +throw new
 EOFException(sm.getString(iib.eof.error));
   }
   }
 +if (standardTimeout  0) {
 +socket.getSocket().setSoTimeout(standardTimeout);
 +}
 +
   inputBuffer.parseRequestLine(false);
 not fully understanding the logic here. But if you ever run into a case
 where standardTimeout=0 and firstReadTimeout=1, then you'd have 1
 millisecond timeout for the parse request line.

Can't happen. Look at the if statement you cut off just above the code
you quoted above.

Mark



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

svn commit: r1095794 - in /tomcat/trunk: java/org/apache/coyote/http11/Http11Processor.java webapps/docs/changelog.xml

[markt]

Author: markt
Date: Thu Apr 21 19:02:46 2011
New Revision: 1095794

URL: http://svn.apache.org/viewvc?rev=1095794view=rev
Log:
Fix TODO - take account of time waiting for a thread when calculating timeouts.

Modified:
tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
tomcat/trunk/webapps/docs/changelog.xml

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=1095794r1=1095793r2=1095794view=diff
==
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Thu Apr 21 
19:02:46 2011
@@ -17,6 +17,7 @@
 
 package org.apache.coyote.http11;
 
+import java.io.EOFException;
 import java.io.IOException;
 import java.io.InterruptedIOException;
 import java.net.InetAddress;
@@ -184,15 +185,48 @@ public class Http11Processor extends Abs
 
 // Parsing the request header
 try {
-//TODO - calculate timeout based on length in queue 
(System.currentTimeMills() - wrapper.getLastAccess() is the time in queue)
+int standardTimeout = 0;
 if (keptAlive) {
 if (keepAliveTimeout  0) {
-socket.getSocket().setSoTimeout(keepAliveTimeout);
+standardTimeout = keepAliveTimeout;
+} else if (soTimeout  0) {
+standardTimeout = soTimeout;
 }
-else if (soTimeout  0) {
-socket.getSocket().setSoTimeout(soTimeout);
+}
+/*
+ * When there is no data in the buffer and this is not the 
first
+ * request on this connection and timeouts are being used the
+ * first read for this request may need a different timeout to
+ * take account of time spent waiting for a processing thread.
+ * 
+ * This is a little hacky but better than exposing the socket
+ * and the timeout info to the InputBuffer
+ */
+if (inputBuffer.lastValid == 0 
+socketWrapper.getLastAccess()  -1 
+standardTimeout  0) {
+
+long queueTime = System.currentTimeMillis() -
+socketWrapper.getLastAccess();
+int firstReadTimeout;
+if (queueTime = standardTimeout) {
+// Queued for longer than timeout but there might be
+// data so use shortest possible timeout
+firstReadTimeout = 1;
+} else {
+// Cast is safe since queueTime must be less than
+// standardTimeout which is an int
+firstReadTimeout = standardTimeout - (int) queueTime;
+}
+socket.getSocket().setSoTimeout(firstReadTimeout);
+if (!inputBuffer.fill()) {
+throw new EOFException(sm.getString(iib.eof.error));
 }
 }
+if (standardTimeout  0) {
+socket.getSocket().setSoTimeout(standardTimeout);
+}
+
 inputBuffer.parseRequestLine(false);
 if (endpoint.isPaused()) {
 // 503 - Service unavailable

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1095794r1=1095793r2=1095794view=diff
==
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Thu Apr 21 19:02:46 2011
@@ -111,6 +111,11 @@
 information was also added to the documentation on how to select an
 appropriate value. 
   /fix
+  fix
+Take account of time spent waiting for a processing thread when
+calculating connection and keep-alive timeouts for the HTTP BIO
+connector. (markt)
+  /fix
 /changelog
   /subsection
   subsection name=Jasper



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

Re: svn commit: r1095794 - in /tomcat/trunk: java/org/apache/coyote/http11/Http11Processor.java webapps/docs/changelog.xml

[Filip Hanik - Dev Lists]

On 4/21/2011 1:02 PM, ma...@apache.org wrote:

+int firstReadTimeout;
+if (queueTime= standardTimeout) {
+// Queued for longer than timeout but there might be
+// data so use shortest possible timeout
+firstReadTimeout = 1;
+} else {
+// Cast is safe since queueTime must be less than
+// standardTimeout which is an int
+firstReadTimeout = standardTimeout - (int) queueTime;
+}
+socket.getSocket().setSoTimeout(firstReadTimeout);
+if (!inputBuffer.fill()) {
+throw new EOFException(sm.getString(iib.eof.error));
  }
  }
+if (standardTimeout  0) {
+socket.getSocket().setSoTimeout(standardTimeout);
+}
+
  inputBuffer.parseRequestLine(false);
not fully understanding the logic here. But if you ever run into a case where standardTimeout=0 and firstReadTimeout=1, then you'd have 1 
millisecond timeout for the parse request line. And the request line, typically comes in one packet, but it is legal to send it up in two. 
And you'd have an invalid SocketTimeoutException here, since 1 is no longer the correct timeout.




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