[SNMP4J] Unnecessary SNMP retries after a response has been received ?

[Frank Fock]

Hi,

Please replace the following method in Snmp.java to
fix the bug:

   public ResponseEvent send(PDU pdu, Target target,
                             TransportMapping transport) throws 
IOException {
     if (!pdu.isConfirmedPdu()) {
       sendMessage(pdu, target, transport);
       return null;
     }
     SyncResponseListener syncResponse = new SyncResponseListener();
     synchronized (syncResponse) {
       PduHandle handle;
       PendingRequest request;
       synchronized (sync) {
         handle = sendMessage(pdu, target, transport);
         request =
             new PendingRequest(handle, syncResponse, target, pdu, target,
                                transport);
         if (logger.isDebugEnabled()) {
           logger.debug("New pending request with handle " + handle);
         }
         pendingRequests.put(handle, request);
         long delay = timeoutModel.getRetryTimeout(0, target.getRetries(),
                                                   target.getTimeout());
         timer.schedule(request, delay);
       }
       try {
         syncResponse.wait();
         PendingRequest retryRequest =
             (PendingRequest) pendingRequests.remove(handle);
         if (retryRequest != null) {
           retryRequest.setFinished();
           retryRequest.cancel();
         }
         if (logger.isDebugEnabled()) {
           logger.debug("Removed pending request with handle: "+handle);
         }
         request.setFinished();
         request.cancel();
       }
       catch (InterruptedException iex) {
         logger.warn(iex);
         // ignore
       }
     }
     return syncResponse.response;
   }


Best regards,
Frank

k j wrote:
> A small program to show the behaviour:
> 
> import junit.framework.TestCase;
> import org.snmp4j.*;
> import org.snmp4j.event.ResponseEvent;
> import org.snmp4j.log.Log4jLogFactory;
> import org.snmp4j.log.LogFactory;
> import org.snmp4j.mp.MPv1;
> import org.snmp4j.smi.OID;
> import org.snmp4j.smi.OctetString;
> import org.snmp4j.smi.UdpAddress;
> import org.snmp4j.smi.VariableBinding;
> import org.snmp4j.transport.DefaultUdpTransportMapping;
> import org.snmp4j.util.DefaultPDUFactory;
> 
> import javax.resource.ResourceException;
> import java.io.IOException;
> import java.net.InetAddress;
> 
> public class Snmp4JTest extends TestCase {
> 
>    private static final OctetString HELLO = new OctetString("hello");
> 
>    /**
>     * Sends one get request to a daemon. Daemon reacts with a delay of 
> 1.5 seconds. Client
>     * is configured with retry 9 (just to show the issue) and timeout 1 
> second.
>     * The test shows that as soon as one retry is send, all retries are 
> send, even if the
>     * response is received after the first retry.
>     */
>    public void testSnmp4jRetryBehavior() throws ResourceException, 
> IOException, InterruptedException {
> 
>        LogFactory.setLogFactory(new Log4jLogFactory());
>        int freePort = 23454;
> 
>        //INIT server
>        UdpAddress udpAddress = new 
> UdpAddress(InetAddress.getByName("127.0.0.1"), freePort);
>        DefaultUdpTransportMapping defaultUdpTransportMapping = new 
> DefaultUdpTransportMapping(udpAddress);
>        Snmp daemon = new Snmp(defaultUdpTransportMapping);
>        final MyCommandResponder myCommandResponder = new 
> MyCommandResponder();
>        myCommandResponder.setSleepBeforeSendingResponse(1500);//force 
> one retry..
> 
>        daemon.addCommandResponder(myCommandResponder);
>        daemon.listen();
> 
>        try {
> 
>            //INIT client
>            DefaultPDUFactory factory = new DefaultPDUFactory();
>            MessageDispatcher mtDispatcher = new MessageDispatcherImpl();
>            mtDispatcher.addMessageProcessingModel(new MPv1());
>            Snmp snmpClient = new Snmp(mtDispatcher, new 
> DefaultUdpTransportMapping());
>            snmpClient.listen();
> 
>            //Create Request
>            Target target = new CommunityTarget(udpAddress, new 
> OctetString("public"));
>            target.setRetries(9);
>            target.setTimeout(1000);
>            PDU pdu = factory.createPDU(target);
>            pdu.setType(PDU.GET);
>            pdu.addAll(new VariableBinding[]{new VariableBinding(new 
> OID("1.2"))});
> 
>            // Send the request
>            ResponseEvent event = snmpClient.send(pdu, target);
>            PDU response = event.getResponse();
>            assertEquals(HELLO, response.get(0).getVariable());
>            System.out.println("Received response: " + response);
>            Thread.sleep(12000);//sleep for a while..
>        } finally {
>            daemon.close();
>        }
> 
>        //server should have received only two requests (request + one 
> retry)
>        assertEquals(2, myCommandResponder.getProcessCount());
> 
>    }
> 
>    private class MyCommandResponder implements CommandResponder {
>        private long m_sleepBeforeSendingResponse = 0;
>        private int m_processCount;
> 
>        public MyCommandResponder(){
>        }
> 
>        public void setSleepBeforeSendingResponse(long 
> sleepBeforeSendingResponse) {
>            m_sleepBeforeSendingResponse = sleepBeforeSendingResponse;
>        }
> 
>        public int getProcessCount() {
>            return m_processCount;
>        }
> 
>        public void processPdu(CommandResponderEvent vEvent) {
>            PDU pdu = vEvent.getPDU();
>            int pduType = pdu.getType();
>            System.out.println("Process PDU: " + pdu);
>            if (pduType == PDU.GET) {
>                m_processCount++;
>                ((VariableBinding) 
> pdu.getVariableBindings().get(0)).setVariable(HELLO);
>            }
>            try {
>                Thread.sleep(m_sleepBeforeSendingResponse);
>            } catch (InterruptedException e) {
>                throw new RuntimeException(e);
>            }
>            pdu.setErrorIndex(0);
>            pdu.setErrorStatus(0);
>            pdu.setType(PDU.RESPONSE);
>            try {
>                
> vEvent.getMessageDispatcher().returnResponsePdu(vEvent.getMessageProcessingModel(), 
> 
>                        vEvent.getSecurityModel(), vEvent.getSecurityName(),
>                        vEvent.getSecurityLevel(), pdu, 
> vEvent.getMaxSizeResponsePDU(), vEvent.getStateReference(),
>                        null);
>            } catch (MessageException e) {
>                throw new RuntimeException(e);
>            }
>        }
>    }
> }
> 
> 
> _______________________________________________
> 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