[SNMP4J] SNMP broadcasts, PDUv1 with Counter64 (in 1.9.1)

[Christof Meerwald]

Hi Frank,

thanks, the fix appears to work.


Christof

Frank Fock wrote:
> Hi Christof,
> 
> Sorry, this seems to be indeed a regression.
> Please try the fix below to fix broadcast
> async request processing:
> 
> Index: Snmp.java
> ===================================================================
> --- Snmp.java    (revision 402)
> +++ Snmp.java    (working copy)
> @@ -1117,17 +1117,7 @@
>        synchronized (pendingRequests) {
>          request = (PendingRequest) pendingRequests.get(handle);
>          if (request != null) {
> -          if (request.isResponseReceived()) {
> -            pendingRequests.remove(handle);
> -            request = null;
> -            if (logger.isInfoEnabled()) {
> -              logger.info("Received second response for request with 
> handle "+
> -                          handle);
> -            }
> -          }
> -          else {
> -            request.responseReceived();
> -          }
> +          request.responseReceived();
>          }
>        }
>        if (request == null) {
> 
> 
> I will consider isolating the Counter64 check into
> a protected method.
> 
> Best regards,
> Frank
> 
> 
> 
> Christof Meerwald wrote:
>> Hi,
>>
>> it appears that with 1.9.1 SNMP broadcasts don't work any more - to be
>> more precise, you now only get the first response (with code that
>> worked perfectly fine with previous snmp4j versions). I think this is
>> related to the following "fix" in 1.9.1:
>>
>> "Fixed: With async request processing and retry>0 there could have been
>> a race condition where a response event had been generated twice for a
>> request."
>>
>> which appears to now only allow a single response event for each
>> request (even if the request is not cancelled in the listener as
>> mentioned in the JavaDoc: "Not canceling a request immediately can be
>> useful when sending a request to a broadcast address.")
>>
>>
>> I also noticed that 1.9.1 now contains a factory interface for creating
>> the PDU class for incoming messages. While this allows one to provide a
>> PDUv1 class that doesn't throw an exception when encountering a
>> Counter64 value, it's still a bit of a hassle as it requires quite a
>> bit of code duplication. I think, snmp4j could quite easily simplify
>> that particular case further by moving the Counter64 check into a
>> separate method. That way, a derived "PDUv1WithCounter64" could easily
>> overwrite that method without having to duplicate any of the decodeBER
>> code. So my proposal would be to add a new protected method to PDUv1:
>>
>> protected void checkVariableBindingSyntax(VariableBinding vb)
>>   throws IOException
>> {
>>   if (vb.getSyntax() == SMIConstants.SYNTAX_COUNTER64) {
>>     throw new MessageException("Counter64 encountered in SNMPv1 PDU
>> "+
>>                                "(RFC 2576 §4.1.2.1)");
>>   }
>> }
>>
>> and call checkVariableBindingSyntax(vb) in decodeBER (line 169).
>>
>> Any thoughts on this?
>>
>>
>> Christof
>>
> 


-- 
Emnico Technologies Ltd - http://www.emnico.com
Dauntsey House, Stonehill Green, Westlea, Swindon SN5 7HB