[SNMP4J] NullPointerException while resending a GET request

Frank Fock fock at agentpp.com
Mon Apr 6 00:08:49 CEST 2009


Hi George,

Synchronizing the PendingRequest.cancel() method causes deadlock
problems. The source problem is the early object release
in that method. The retry creation phase can be protected by
a simple flag that is true (on) while the next retry is being
prepared and not yet sent.

Please replace the PendingRequest class in Snmp.java with the
code below (or the next nightly build) and let me know if
the NPE goes away:

Best regards,
Frank


   class PendingRequest extends TimerTask implements PduHandleCallback {

     private PduHandle key;
     protected int retryCount;
     protected ResponseListener listener;
     protected Object userObject;

     protected PDU pdu;
     protected Target target;
     protected TransportMapping transport;

     private int requestStatus = 0;
     // Maximum request status - allows to receive up to two reports and 
then
     // send the original request again. A value of 0 is used for discovery.
     private int maxRequestStatus = 2;

     private volatile boolean finished = false;
     private volatile boolean responseReceived = false;

     private volatile boolean pendingRetry = false;


     public PendingRequest(ResponseListener listener,
                           Object userObject,
                           PDU pdu,
                           Target target,
                           TransportMapping transport) {
       this.userObject = userObject;
       this.listener = listener;
       this.retryCount = target.getRetries();
       this.pdu = pdu;
       this.target = (Target) target.clone();
       this.transport = transport;
     }

     private PendingRequest(PendingRequest other) {
       other.pendingRetry = true;
       this.userObject = other.userObject;
       this.listener = other.listener;
       this.retryCount = other.retryCount - 1;
       this.pdu = other.pdu;
       this.target = other.target;
       this.requestStatus = other.requestStatus;
       this.responseReceived = other.responseReceived;
       this.transport = other.transport;
     }

     protected void registerRequest(PduHandle handle) {

     }

     public void responseReceived() {
       this.responseReceived = true;
     }

     public synchronized void pduHandleAssigned(PduHandle handle, Object 
pdu) {
       if (key == null) {
         key = handle;
         pendingRequests.put(handle, this);
         registerRequest(handle);
         if (logger.isDebugEnabled()) {
           logger.debug("Running pending "+
                        ((listener instanceof SyncResponseListener) ?
                         "sync" : "async")+
                        " request with handle " + handle+
                        " and retry count left "+retryCount);
         }
         long delay = timeoutModel.getRetryTimeout(target.getRetries() -
                                                   retryCount,
                                                   target.getRetries(),
                                                   target.getTimeout());
         if ((!finished) && (!responseReceived)) {
           timer.schedule(this, delay);
         }
       }
     }

     /**
      * Process retries of a pending request.
      */
     public synchronized void run() {
       PduHandle m_key = key;
       PDU m_pdu = pdu;
       Target m_target = target;
       TransportMapping m_transport = transport;
       ResponseListener m_listener = listener;
       Object m_userObject = userObject;

       if ((m_key == null) || (m_pdu == null) || (m_target == null) ||
           (m_listener == null)) {
         if (logger.isDebugEnabled()) {
           logger.debug("PendingRequest canceled key="+m_key+", pdu="+m_pdu+
               ", target="+m_target+", transport="+m_transport+", 
listener="+
               m_listener);
         }
         return;
       }

       try {
         boolean retry;
         synchronized (pendingRequests) {
           retry = (!finished) && (retryCount > 0) && (!responseReceived);
         }
         if (retry) {
           try {
             PendingRequest nextRetry = new PendingRequest(this);
             sendMessage(m_pdu, m_target, m_transport, nextRetry);
             this.pendingRetry = false;
           }
           catch (IOException ex) {
             finished = true;
             logger.error("Failed to send SNMP message to " + m_target +
                          ": " +
                          ex.getMessage());
             messageDispatcher.releaseStateReference(m_target.getVersion(),
                 m_key);
             listener.onResponse(new ResponseEvent(Snmp.this, null,
                                                   m_pdu, null, 
m_userObject,
                                                   ex));
           }
         }
         else if (!finished) {
           finished = true;
           pendingRequests.remove(m_key);

           if (!responseReceived) {
             // request timed out
             if (logger.isDebugEnabled()) {
               logger.debug("Request timed out: " + 
m_key.getTransactionID());
             }
             messageDispatcher.releaseStateReference(m_target.getVersion(),
                                                     m_key);
             listener.onResponse(new ResponseEvent(Snmp.this, null,
                                                   m_pdu, null, 
m_userObject));
           }
         }
         else {
           // make sure pending request is removed even if response listener
           // failed to call Snmp.cancel
           pendingRequests.remove(m_key);
         }
       }
       catch (RuntimeException ex) {
         logger.error("Failed to process pending request " + m_key +
                      " because " + ex.getMessage(), ex);
         throw ex;
       }
       catch (Error er) {
         logger.fatal("Failed to process pending request " + m_key +
                      " because " + er.getMessage(), er);
         throw er;
       }
     }

     public boolean setFinished() {
       boolean currentState = finished;
       this.finished = true;
       return currentState;
     }

     public void setMaxRequestStatus(int maxRequestStatus) {
       this.maxRequestStatus = maxRequestStatus;
     }

     public int getMaxRequestStatus() {
       return maxRequestStatus;
     }

     public boolean isResponseReceived() {
       return responseReceived;
     }

     /**
      * Cancels the request and clears all internal fields by setting them
      * to <code>null</code>.
      * @return
      *    <code>true</code> if cancellation was successful.
      */
     public boolean cancel(){
         boolean result = super.cancel();

         // free objects early
         if (!pendingRetry) {
           key = null;
           pdu = null;
           target = null;
           transport = null;
           listener = null;
           userObject = null;
         }
         return result;
     }
   }


George Tsaloukidis wrote:
> Dear Frank,
> 
> I am using snmp4j version 1.9.3d for executing synchronous snmp GET requests.
> My application has many threads, but each thread uses its own org.snmp4j.Snmp instance.
> Sometimes I get the following exception:
> 
> java.lang.NullPointerException
>         at org.snmp4j.Snmp$PendingRequest.pduHandleAssigned(Unknown Source)
>         at org.snmp4j.MessageDispatcherImpl.sendPdu(Unknown Source)
>         at org.snmp4j.Snmp.sendMessage(Unknown Source)
>         at org.snmp4j.Snmp$PendingRequest.run(Unknown Source)
>         at java.util.TimerThread.mainLoop(Timer.java:512)
>         at java.util.TimerThread.run(Timer.java:462)
> Exception in thread "Timer-4877612" java.lang.NullPointerException
>         at org.snmp4j.Snmp$PendingRequest.pduHandleAssigned(Unknown Source)
>         at org.snmp4j.MessageDispatcherImpl.sendPdu(Unknown Source)
>         at org.snmp4j.Snmp.sendMessage(Unknown Source)
>         at org.snmp4j.Snmp$PendingRequest.run(Unknown Source)
>         at java.util.TimerThread.mainLoop(Timer.java:512)
>         at java.util.TimerThread.run(Timer.java:462)
> 
> After rebuilding SNMP4J.jar with debug information I saw that the problem was at line 1449 of Snmp.java file:
> 
>         long delay = timeoutModel.getRetryTimeout(target.getRetries() -
>                                                   retryCount,
>                                                   target.getRetries(),
>                                                   target.getTimeout());
> 
> That means the target object was null while trying to retransmit the request.
> That can happen only for two reasons: a) PendingRequest of the retry action created
> just after the cancellation of the very first PendingRequest (line 1487) or,
> the PendingRequest canceled while inside the pduHandleAssigned() method.
> The second case can appear only if the main thread gets that object from the pendingRequests list (line 811).
> That means the Timer thread had previously managed to execute the 1440 line and suspended for a while.
> 
> The problem is that the send() method (line 793) can call the cancel() operation on a PendingRequest
> at any time, without taking into consideration if the TimerThread is executed.
> 
> A solution can be to first lock the PendingRequest before calling setFinished() and cancel() methods.
> Line 811 should also go after the cancellation of the first request or else if PendingRequest.run() has started,
> the newly created PendingRequest is left into the pendingRequests list for ever.
> I have tested the above solution and I had no problem.
> 
> And something else. At line 810 the wait() method is called.
> Sun advices such calls to be wrapped into a while loop
> (see http://java.sun.com/docs/books/jls/third_edition/html/memory.html#17.8.1 and 
> http://java.sun.com/javase/6/docs/api/java/lang/Object.html#wait() links)
> 
> while(syncResponse.response == null)
> {
>    syncResponse.wait();
> }
> 
> Best regards,
> 
> George Tsaloukidis,
> Senior Software Engineer
> Systems Software Developement
> Intracom Telecom S.A
> _______________________________________________
> SNMP4J mailing list
> SNMP4J at agentpp.org
> http://lists.agentpp.org/mailman/listinfo/snmp4j

-- 
AGENT++
http://www.agentpp.com
http://www.mibexplorer.com
http://www.mibdesigner.com




More information about the SNMP4J mailing list