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