Oh yes, you are correct and I am wrong ! I apologize. But then we should catch IOException or ClosedChannelException ? And do the same for the other uses of 'socket' in this function ?
> From: Paul Marquis [mailto:[email protected]] (snip) > > If another thread closes where you say, you should get an > IOException or SocketException, not a NullPointerException, > when receive() is called. The socket member variable may > change to null, but socketCopy will not. > > On Sep 27, 2010, at 3:38 AM, Tosoni wrote: > > > Sorry guys, but I don't believe your tries fix the race for real. > > > > The problem is that a socket.close() can occur just before the > > socket.receive() > > If you just test the socket value, this does not forbit a > > socket.close() > > happening *after* > > your test and *before* the socket.receive().> > try { > >>> DatagramSocket socketCopy = socket; > >>> if (socketCopy == null) { > >>> stop = true; > >>> continue; > >>> } > > *** what if another thread closes at this point ? *** > >>> socketCopy.receive(packet); > >>> } > > > > The real fix must use some semaphore or mutex which embraces: > > on one hand, both the test of 'socket==null' and the receive() > > on the other hand, both the change 'socket=null' and the close() > > > > My own try (I do not suggest it as a correct fix) is a quick-and- > > dirty: > > try { > > socket.receive(packet); > > } > > catch (InterruptedIOException iiox) { > > if (iiox.bytesTransferred <= 0) { > > continue; > > } > > } > > + // UGLY FIX: socket close/receive synchro > > + catch (NullPointerException iiox) { > > + // This handles the close race (stop=false but > > socket=null) > > + continue; > > + } > > > > > > Hmm, also, there is another race condition which happens > sometimes, > > here: > > public void run() { > > try { > > here===> socket.setSoTimeout(getSocketTimeout()); > > Really I guess it can happen everywhere 'socket' is used in a thread > > different than the closing thread... > > > > Regards > > > >> -----Original Message----- > >> From: [email protected] [mailto:snmp4j- > >> [email protected]]on > >> Behalf Of Paul Marquis > >> Sent: Friday, September 24, 2010 1:32 AM > >> To: Frank Fock > >> Cc: [email protected] > >> Subject: Re: [SNMP4J] Another race condition fix > >> forDefaultUdpTransportMapping > >> > >> > >> Thanks for fixing this! > >> > >> Any idea when there will be an official release with this fix > >> in it? > >> I just joined the list, so if you've already discussed > when the next > >> release will be available, I apologize. > >> > >> On Sep 23, 2010, at 7:24 PM, Frank Fock wrote: > >> > >>> Hi Paul, > >>> > >>> Thank you for the bug report. If fixed it a bit different: > >>> > >>> try { > >>> DatagramSocket socketCopy = socket; > >>> if (socketCopy == null) { > >>> stop = true; > >>> continue; > >>> } > >>> socketCopy.receive(packet); > >>> } > >>> > >>> Best regards, > >>> Frank > >>> > >>> On 23.09.2010 23:44, Paul Marquis wrote: > >>>> Version 1.11.1 of SNMP4J included a fix for a race condition in > >>>> DefautlUdpTrasnportMapping which caused a NullPointerException. > >>>> However, there is still one that remains while > attempting to read a > >>>> packet and I see this from time to time. Below is a patch > >> that fixes > >>>> the problem for me that I'd like to submit for review. > >>>> > >>>> Index: src/org/snmp4j/transport/DefaultUdpTransportMapping.java > >>>> > =================================================================== > >>>> --- > >> src/org/snmp4j/transport/DefaultUdpTransportMapping.java > >>>> (revision > >>>> 215) > >>>> +++ > >> src/org/snmp4j/transport/DefaultUdpTransportMapping.java (working > >>>> copy) > >>>> @@ -337,7 +337,11 @@ > >>>> > >>>> udpAddress.getPort()); > >>>> try { > >>>> try { > >>>> - socket.receive(packet); > >>>> + DatagramSocket readingSocket = socket; > >>>> + if (readingSocket == null) { > >>>> + continue; > >>>> + } > >>>> + readingSocket.receive(packet); > >>>> } > >>>> catch (InterruptedIOException iiox) { > >>>> if (iiox.bytesTransferred<= 0) { > >>>> > >>>> _______________________________________________ > >>>> SNMP4J mailing list > >>>> [email protected] > >>>> http://lists.agentpp.org/mailman/listinfo/snmp4j > >>> > >>> -- > >>> AGENT++ > >>> http://www.agentpp.com > >>> http://www.snmp4j.com > >>> http://www.mibexplorer.com > >>> http://www.mibdesigner.com > >>> > >>> _______________________________________________ > >>> SNMP4J mailing list > >>> [email protected] > >>> http://lists.agentpp.org/mailman/listinfo/snmp4j > >> > >> _______________________________________________ > >> SNMP4J mailing list > >> [email protected] > >> http://lists.agentpp.org/mailman/listinfo/snmp4j > >> > > > > _______________________________________________ SNMP4J mailing list [email protected] http://lists.agentpp.org/mailman/listinfo/snmp4j
