[SNMP4J] SocketTimeout task leaks open socket

[Bruno Abreu]

Hello,

we are using snmp4j to handle NTCIP-PMPP protocol which is based in SNMP over
TCP.

More details about NTCIP-PMPP specification, see:
http://www.ntcip.org/library/documents/pdf/pmpp01.pdf

In our case, we need to connect to a device that can only accept one
connection at a time. This lead us to find what we think might be a bug in the
DefaultTcpTransportMapping class.

Because of that "one connection at a time" requirement, we need to close the
socket explicitly, immediately after communication with the device is
finished, so that other clients can connect. The SocketTimeout timer task,
however, is still scheduled to run after the socket is closed. It will
eventually execute and, since it holds a reference to a socket that is already
closed, invoking close() on that socket will cause no harm. The problem is
that it also removes from the 'sockets' Hashtable any entry that might have
been created with the same target address of the socket it was supposed to be
closing.

Since it's possible that a new conversation to the same device is already
taking place, when a new message is about to be sent out, the
DefaultTcpTransportMapping doesn't find the entry in the 'sockets' table and
tries to establish a new connection. From that point on we receive Connection
refused exceptions.

The following patch succeeded in solving our problem. We submit it here in
case you want to consider applying to the DefaultTcpTransportMapping class in
a future release.

»»»
--- DefaultTcpTransportMapping.java.2.5.11	2018-01-26 11:36:14.019589314 +0000
+++ DefaultTcpTransportMapping.java	2018-01-26 11:11:42.000000000 +0000
@@ -258,6 +258,9 @@ public class DefaultTcpTransportMapping
     }
     SocketEntry entry = sockets.remove(remoteAddress);
     if (entry != null) {
+      if (entry.getSocketCleaner() != null) {
+        entry.getSocketCleaner().cancel();
+      }
       Socket s = entry.getSocket();
       if (s != null) {
         SocketChannel sc = entry.getSocket().getChannel();
@@ -388,7 +391,9 @@ public class DefaultTcpTransportMapping

   private synchronized void timeoutSocket(SocketEntry entry) {
     if (connectionTimeout > 0) {
-      socketCleaner.schedule(new SocketTimeout(entry), connectionTimeout);
+      SocketTimeout socketTimeoutCleaner = new SocketTimeout(entry);
+      entry.setSocketCleaner(socketTimeoutCleaner);
+      socketCleaner.schedule(socketTimeoutCleaner, connectionTimeout);
     }
   }

@@ -421,6 +426,7 @@ public class DefaultTcpTransportMapping
     private ByteBuffer readBuffer = null;
     private volatile int registrations = 0;
     private volatile int busyLoops = 0;
+    private SocketTimeout socketCleaner = null;

     public SocketEntry(TcpAddress address, Socket socket) {
       this.peerAddress = address;
@@ -506,6 +512,14 @@ public class DefaultTcpTransportMapping
       busyLoops = 0;
     }

+    public SocketTimeout getSocketCleaner() {
+        return socketCleaner;
+    }
+
+    public void setSocketCleaner(SocketTimeout socketCleaner) {
+        this.socketCleaner = socketCleaner;
+    }
+
     public String toString() {
       return "SocketEntry[peerAddress="+peerAddress+
           ",socket="+socket+",lastUse="+new
Date(lastUse/SnmpConstants.MILLISECOND_TO_NANOSECOND)+
@@ -592,7 +606,9 @@ public class DefaultTcpTransportMapping
       if (logger.isDebugEnabled()) {
         logger.debug("Scheduling " + nextRun);
       }
-      socketCleaner.schedule(new SocketTimeout(entry), nextRun);
+      SocketTimeout socketTimeoutCleaner = new SocketTimeout(entry);
+      entry.setSocketCleaner(socketTimeoutCleaner);
+      socketCleaner.schedule(socketTimeoutCleaner, nextRun);
     }

     public boolean cancel(){
«««


That you for you attention,
Bruno Abreu

-- 
Living Data - Sistemas de Informação e Apoio à Decisão, Lda.
LxFactory - Rua Rodrigues de Faria, 103, edifício I, 4º piso
1300-501 LISBOA                       Phone:  +351 213622163
Portugal                              URL: www.livingdata.pt