[SNMP4J] Another race condition fix forDefaultUdpTransportMapping

[Tosoni]

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
>