From 2204d5835f5d46d0dad017b601639a8226931b35 Mon Sep 17 00:00:00 2001
From: bimalyn-IBM <bimalyn@us.ibm.com>
Date: Tue, 30 May 2017 11:15:36 -0500
Subject: [PATCH] Fix: SNMP fails to close/destroy sessions when it errors
 (#318)

* I've modified the snmp libraries so that the server and the community can be defined by the msg.
Error will be thrown if you try to override what was defined in the node.
Verified that the contents of msg is no longer clobbered.

Signed-off-by: Bryan Malyn <bimalyn@us.ibm.com>

* Replaced tabs with 4 spaces.
Signed-off-by: Bryan Malyn <bimalyn@us.ibm.com>

* fix extra spacing

Signed-off-by: Bryan Malyn <bimalyn@us.ibm.com>

* Standardise node.error to include msg so that errors can caught

Signed-off-by: Bryan Malyn <bimalyn@us.ibm.com>

* Update documentation for snmp nodes

fixed documentation as noted, removed conflict and iff from documention
remove node.warns if host or community are set in both the node config and msg

Signed-off-by: Bryan Malyn <bimalyn@us.ibm.com>

* FIX: Close net-snmp sessions
Signed-off-by: Bryan Malyn <bimalyn@us.ibm.com>

* Fix 2: The previous push did not cleanly address the problem.

This commit uses a singleton aproach to create socket.
It still needs to be tested to see if there is any issue to the never-close,
but reuse socket model. My only concern is if a socket dies, do we need to do something to reestablish it?

Signed-off-by: Bryan Malyn <bimalyn@us.ibm.com>
---
 io/snmp/snmp.js | 46 ++++++++++++++--------------------------------
 1 file changed, 14 insertions(+), 32 deletions(-)

diff --git a/io/snmp/snmp.js b/io/snmp/snmp.js
index a1159ac0..3bfc25fa 100644
--- a/io/snmp/snmp.js
+++ b/io/snmp/snmp.js
@@ -3,6 +3,16 @@ module.exports = function(RED) {
     "use strict";
     var snmp = require("net-snmp");
 
+    var sessions = {};
+
+    function getSession(host, community, version){
+        var sessionKey = host + ":" + community + ":" + version;
+        if (!(sessionKey in sessions)){
+            sessions[sessionKey] = snmp.createSession(host, community, {version: version});
+        }
+        return sessions[sessionKey];
+    }
+
     function SnmpNode(n) {
         RED.nodes.createNode(this,n);
         this.community = n.community;
@@ -16,8 +26,7 @@ module.exports = function(RED) {
             var community = node.community || msg.community;
             var oids = node.oids || msg.oid;
             if (oids) {
-                node.session = snmp.createSession(host, community, {version: node.version});
-                node.session.get(oids.split(","), function(error, varbinds) {
+                getSession(host, community, node.version).get(oids.split(","), function(error, varbinds) {
                     if (error) {
                         node.error(error.toString(),msg);
                     }
@@ -42,12 +51,6 @@ module.exports = function(RED) {
                 node.warn("No oid(s) to search for");
             }
         });
-
-        this.on("close", function() {
-            if (node.session) {
-                node.session.close();
-            }
-        });
     }
     RED.nodes.registerType("snmp",SnmpNode);
 
@@ -72,8 +75,7 @@ module.exports = function(RED) {
             var oids = node.oids || msg.oid;
             if (oids) {
                 msg.oid = oids;
-                node.session = snmp.createSession(host, community, {version: node.version});
-                node.session.table(oids, maxRepetitions, function(error, table) {
+                getSession(host, community, node.version).table(oids, maxRepetitions, function(error, table) {
                     if (error) {
                         node.error(error.toString(), msg);
                     }
@@ -107,12 +109,6 @@ module.exports = function(RED) {
                 node.warn("No oid to search for");
             }
         });
-
-        this.on("close", function() {
-            if (node.session) {
-                node.session.close();
-            }
-        });
     }
     RED.nodes.registerType("snmp table",SnmpTNode);
 
@@ -144,8 +140,7 @@ module.exports = function(RED) {
             var oids = node.oids || msg.oid;
             if (oids) {
                 msg.oid = oids;
-                node.session = snmp.createSession(host, community, {version: node.version});
-                node.session.subtree(msg.oid, maxRepetitions, feedCb, function(error) {
+                getSession(host, community, node.version).subtree(msg.oid, maxRepetitions, feedCb, function(error) {
                     if (error) {
                         node.error(error.toString(), msg);
                     }
@@ -161,12 +156,6 @@ module.exports = function(RED) {
                 node.warn("No oid to search for");
             }
         });
-
-        this.on("close", function() {
-            if (node.session) {
-                node.session.close();
-            }
-        });
     }
     RED.nodes.registerType("snmp subtree",SnmpSubtreeNode);
 
@@ -199,8 +188,7 @@ module.exports = function(RED) {
             var community = node.community || msg.community;
             if (oids) {
                 msg.oid = oids;
-                node.session = snmp.createSession(host, community, {version: node.version});
-                node.session.walk(msg.oid, maxRepetitions, feedCb, function(error) {
+                getSession(host, community, node.version).walk(msg.oid, maxRepetitions, feedCb, function(error) {
                     if (error) {
                         node.error(error.toString(), msg);
                     }
@@ -216,12 +204,6 @@ module.exports = function(RED) {
                 node.warn("No oid to search for");
             }
         });
-
-        this.on("close", function() {
-            if (node.session) {
-                node.session.close();
-            }
-        });
     }
     RED.nodes.registerType("snmp walker",SnmpWalkerNode);
 };