2012/1/4 Konstantin Kolinko <knst.koli...@gmail.com>: > > 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.
and I think that if() might be a cause of missing some ERROR events in comet. > > 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. > One more oddity with r944518 in AprEndpoint: 1. I do not understand why one has to set TCCL before calling getExecutor().execute(). We do not do this in other places. E.g. we do not do it in AprEndpoint#processSocket(long). 2. r944518 says about TCK failures, but TCK does not test comet and so it could not be a reason for the change in this method which is used only by comet. So it looks that not only the if(), but the whole r944518 change in AprEndpoint has to be reverted. Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
