[snmp] Fix memory leak in SNMP (#8672)

* Fix memory leak in SNMP. When receiving an async request it is placed in a pending request queue. Not canceling it leads to infinty life cycle for those requests, filling up memory.
* Test the canceling of the snmp aync request by its handler.

Signed-off-by: Falk Bauer <falk.bauer@k-is.com>

diff --git a/bundles/org.openhab.binding.snmp/src/main/java/org/openhab/binding/snmp/internal/SnmpTargetHandler.java b/bundles/org.openhab.binding.snmp/src/main/java/org/openhab/binding/snmp/internal/SnmpTargetHandler.java
index 29369774b1431d6d8e32e3eb219f7e8e656bea84..4d37c589e3e8f57229c5c04cfbfadb47420adbe6 100644 (file)
--- a/bundles/org.openhab.binding.snmp/src/main/java/org/openhab/binding/snmp/internal/SnmpTargetHandler.java
+++ b/bundles/org.openhab.binding.snmp/src/main/java/org/openhab/binding/snmp/internal/SnmpTargetHandler.java
@@ -52,6 +52,7 @@ import org.snmp4j.CommandResponderEvent;
 import org.snmp4j.CommunityTarget;
 import org.snmp4j.PDU;
 import org.snmp4j.PDUv1;
+import org.snmp4j.Snmp;
 import org.snmp4j.event.ResponseEvent;
 import org.snmp4j.event.ResponseListener;
 import org.snmp4j.mp.SnmpConstants;
@@ -175,6 +176,15 @@ public class SnmpTargetHandler extends BaseThingHandler implements ResponseListe
         if (event == null) {
             return;
         }
+
+        if (event.getSource() instanceof Snmp) {
+            // Always cancel async request when response has been received
+            // otherwise a memory leak is created! Not canceling a request
+            // immediately can be useful when sending a request to a broadcast
+            // address (Comment is taken from the snmp4j API doc).
+            ((Snmp) event.getSource()).cancel(event.getRequest(), this);
+        }
+
         PDU response = event.getResponse();
         if (response == null) {
             Exception e = event.getError();

diff --git a/bundles/org.openhab.binding.snmp/src/test/java/org/openhab/binding/snmp/internal/SnmpTargetHandlerTest.java b/bundles/org.openhab.binding.snmp/src/test/java/org/openhab/binding/snmp/internal/SnmpTargetHandlerTest.java
index ec500d79dce181cb9a4e95fa2d0125fccf8b88cb..16b82435edee4bc032ba0aadfd711cf66a5242ff 100644 (file)
--- a/bundles/org.openhab.binding.snmp/src/test/java/org/openhab/binding/snmp/internal/SnmpTargetHandlerTest.java
+++ b/bundles/org.openhab.binding.snmp/src/test/java/org/openhab/binding/snmp/internal/SnmpTargetHandlerTest.java
@@ -24,6 +24,7 @@ import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.library.types.OnOffType;
 import org.openhab.core.library.types.StringType;
 import org.snmp4j.PDU;
+import org.snmp4j.Snmp;
 import org.snmp4j.event.ResponseEvent;
 import org.snmp4j.smi.Counter64;
 import org.snmp4j.smi.Integer32;
@@ -104,4 +105,27 @@ public class SnmpTargetHandlerTest extends AbstractSnmpTargetHandlerTest {
         thingHandler.onResponse(event);
         verify(thingHandlerCallback, atLeast(1)).stateUpdated(eq(CHANNEL_UID), eq(new DecimalType("12.4")));
     }
+
+    @Test
+    public void testCancelingAsyncRequest() {
+        setup(SnmpBindingConstants.CHANNEL_TYPE_UID_NUMBER, SnmpChannelMode.READ, SnmpDatatype.FLOAT);
+        PDU responsePDU = new PDU(PDU.RESPONSE,
+                Collections.singletonList(new VariableBinding(new OID(TEST_OID), new OctetString("12.4"))));
+
+        SnmpMock source = new SnmpMock();
+
+        ResponseEvent event = new ResponseEvent(source, null, null, responsePDU, null);
+
+        thingHandler.onResponse(event);
+        assertEquals(1, source.cancelCallCounter);
+    }
+
+    class SnmpMock extends Snmp {
+        public int cancelCallCounter = 0;
+
+        @Override
+        public void cancel(PDU request, org.snmp4j.event.ResponseListener listener) {
+            ++cancelCallCounter;
+        }
+    }
 }