tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 46985] Impossible condition in coyote.http11.Http11Processor.process(Socket socket)
Date Mon, 18 May 2009 22:10:31 GMT
https://issues.apache.org/bugzilla/show_bug.cgi?id=46985





--- Comment #2 from Konstantin Kolinko <knst.kolinko@gmail.com>  2009-05-18 15:10:29
PST ---
For reference: Mark's fix in trunk is r763262 ( https://svn.apache.org/viewcvs.cgi?view=rev&rev=763262
)

(In reply to comment #0)
> coyote.http11.Http11Processor.process(Socket socket)
> ...
> 
>         int soTimeout = socket.getSoTimeout();
>         int oldSoTimeout = soTimeout;
> 
>         int threadRatio = (endpoint.getCurrentThreadsBusy() * 100)
>                 / endpoint.getMaxThreads();
>         if (threadRatio > 75) {
>             keepAliveLeft = 1;
>         }
> 
>         if (soTimeout != oldSoTimeout) {
> 
> The above condition will never be true. It looks like the code is trying to
> reset the timeout if it has not changed, but it will never do so.
> 

That "if (soTimeout != oldSoTimeout) { .. }" works in TC 5.5, because of some
block of code that precedes it, but is dead in TC 6.0 because that preceding
block is removed. 

You can look at TC 5.5 sources, but as I was studying it through svn log, I
will give a reference to annotated source of it at revision 398045 ( https://svn.apache.org/viewcvs.cgi?view=rev&rev=398045
):

http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java?annotate=398045&limit_changes=0&pathrev=423920

The code was the following:

751 :                  int soTimeout = socket.getSoTimeout(); 
752 :                  int oldSoTimeout = soTimeout;
753 :                 
754 :     remm     396579     int threadRatio =
(endpoint.getCurrentThreadsBusy() * 100)
755 :     remm     389146     / endpoint.getMaxThreads();
756 :                 if ((threadRatio > 33) && (threadRatio <= 66)) {
757 :                 soTimeout = soTimeout / 2;
758 :                 } else if ((threadRatio > 66) && (threadRatio <= 90))
{
759 :                 soTimeout = soTimeout / 3;
760 :                 keepAliveLeft = 1;
761 :                 } else if (threadRatio > 90) {
762 :                 soTimeout = soTimeout / 20;
763 :                 keepAliveLeft = 1;
764 :                 }
765 :                 
766 :                 if (soTimeout != oldSoTimeout) {

I will propose removal of the dead code in TC 6.0.

> Both the method and the class have a variable called "socket" which may be part
> of the problem - is the method trying to set the instance socket to have the
> same timeout as the parameter socket, or vice versa?
> 
> The socket parameter should be renamed.
> 

No error there, no need to rename.

Both are pointing to the same object. Also, you may note, that
this.socket is set back to null at the end of the method.

> Note that the Javadoc appears to be completely wrong as well.

Oh, it says about input and output streams. Those are provided by the socket.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

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


Mime
View raw message