2012/1/4 Mark Thomas <ma...@apache.org>:
> On 03/01/2012 21:32, Mark Thomas wrote:
>> On 03/01/2012 21:26, André Warnier wrote:
>>> Mark Thomas wrote:
>>>> I am trying to bring together all the information I have gleaned on this
>>>> so far from the multiple threads to try and find the common factors.
>>
>> <snip/>
>>
>>> Suggestion: the "large POST requests" mentioned by a couple of posters
>>> suggest file uploads, which may imply temporary files used to buffer the
>>> files being uploaded, no ?
>>
>> Probably not. That code is connector agnostic. There might be something
>> in the APR InputBuffer though.
>
> The APR InputBuffer is untouched between 7.0.21 and 7.0.22. However, I
> did find an error in r1166072 that might explain the fd leak. I am
> trying to reproduce it at the moment. In the meantime, if someone could
> try the following patch that would be great:
>
> Index: java/org/apache/tomcat/util/net/AprEndpoint.java
> ===================================================================
> --- java/org/apache/tomcat/util/net/AprEndpoint.java    (revision 1226551)
> +++ java/org/apache/tomcat/util/net/AprEndpoint.java    (working copy)
> @@ -1364,7 +1364,6 @@
>                         } else {
>                             destroySocket(desc[n*2+1]);
>                         }
> -                        return true;
>                     }
>                 }
>             } else if (rv < 0) {
>
>

Good catch.

1. I think to reproduce the leak here the Poll.pol() has to return
several sockets where one has Poll.APR_POLLHUP flag set. The sockets
after that one will be lost.

(I do not see an easy way to get processSocket() to return false, so I
think APR_POLLHUP is a more easy target to reproduce this.).

2. The
processSocket(desc[n*2+1], SocketStatus.DISCONNECT);
call just above the fixed line.

It looks like a NOOP, because processSocket(long,SocketStatus) has an
if() that does not mention SocketStatus.DISCONNECT.

I think it is one more way to leak sockets. This method is used only
in comet requests.

Can the "if()" condition in processSocket(long,SocketStatus) be removed?
Besides SocketStatus.DISCONNECT. there is one more status that is not
mentioned in that if() and can lead to similar leaks:
SocketStatus.ERROR.

The if() was added in
http://svn.apache.org/viewvc?view=revision&revision=944518

I do not see a reason for this if(). The r944518 says about "running
with a SecurityManager" so it should have just added a
PrivilegedAction there.


Best regards,
Konstantin Kolinko

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

Reply via email to