[SNMP4J] Another race condition fix forDefaultUdpTransportMapping

[Tosoni]

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:paul at viridity.com]
(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: snmp4j-bounces at agentpp.org [mailto:snmp4j- 
> >> bounces at agentpp.org]On
> >> Behalf Of Paul Marquis
> >> Sent: Friday, September 24, 2010 1:32 AM
> >> To: Frank Fock
> >> Cc: snmp4j at agentpp.org
> >> 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
> >>>> SNMP4J at agentpp.org
> >>>> 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
> >>> SNMP4J at agentpp.org
> >>> http://lists.agentpp.org/mailman/listinfo/snmp4j
> >>
> >> _______________________________________________
> >> SNMP4J mailing list
> >> SNMP4J at agentpp.org
> >> http://lists.agentpp.org/mailman/listinfo/snmp4j
> >>
> >
> 
>