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 2011/04/21 21:02:46 UTC
svn commit: r1095794 - in /tomcat/trunk:
java/org/apache/coyote/http11/Http11Processor.java webapps/docs/changelog.xml
Author: markt
Date: Thu Apr 21 19:02:46 2011
New Revision: 1095794
URL: http://svn.apache.org/viewvc?rev=1095794&view=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=1095794&r1=1095793&r2=1095794&view=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=1095794&r1=1095793&r2=1095794&view=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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1095794 - in /tomcat/trunk: java/org/apache/coyote/http11/Http11Processor.java
webapps/docs/changelog.xml
Posted by Mark Thomas <ma...@apache.org>.
On 21/04/2011 20:25, Filip Hanik - Dev Lists wrote:
> On 4/21/2011 1:02 PM, markt@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1095794 - in /tomcat/trunk: java/org/apache/coyote/http11/Http11Processor.java
webapps/docs/changelog.xml
Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
On 4/21/2011 1:02 PM, markt@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org