On 26/10/2011 13:41, Konstantin Kolinko wrote: > 2011/10/23 <ma...@apache.org>: >> Author: markt >> Date: Sat Oct 22 23:24:31 2011 >> New Revision: 1187826 >> >> URL: http://svn.apache.org/viewvc?rev=1187826&view=rev >> Log: >> Fix some low-hanging FindBugs fruit >> >> Modified: >> tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java >> >> tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java >> >> tomcat/trunk/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestConcurrency.java >> tomcat/trunk/test/org/apache/catalina/comet/TestCometProcessor.java >> >> tomcat/trunk/test/org/apache/catalina/tribes/group/interceptors/TestNonBlockingCoordinator.java >> >> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1187826&r1=1187825&r2=1187826&view=diff >> ============================================================================== >> --- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original) >> +++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Sat Oct 22 >> 23:24:31 2011 >> @@ -1565,7 +1565,7 @@ public class NioEndpoint extends Abstrac >> if (ka!=null) ka.setComet(false); >> socket.getPoller().cancelledKey(key, >> SocketStatus.ERROR, false); >> } >> - if (socket!=null) nioChannels.offer(socket); >> + nioChannels.offer(socket); >> socket = null; >> if ( ka!=null ) keyCache.offer(ka); >> ka = null; >> @@ -1579,7 +1579,7 @@ public class NioEndpoint extends Abstrac >> ka = (KeyAttachment) key.attachment(); >> socket.getPoller().cancelledKey(key, >> SocketStatus.DISCONNECT, false); >> } >> - if (socket!=null) nioChannels.offer(socket); >> + nioChannels.offer(socket); >> socket = null; >> if ( ka!=null ) keyCache.offer(ka); >> ka = null; > > The "socket" here is not a local variable, but a field. > Is it true that the null check can be removed here?
I believe so yes. I checked that before I made the change. > I think it would be safer with null checks. I'd rather see some analysis that shows we need them but I'm not against this without the analysis. > The "synchronized (socket)" that wraps it all is a bit odd with > socket=null assignments inside of it, but IIRC that will work. Agreed. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
