From 5a326369baa7f9612f0dab1ff4f9e78d149d253c Mon Sep 17 00:00:00 2001
From: pavel-shirshov <pavelsh@microsoft.com>
Date: Wed, 27 May 2020 12:58:42 -0700
Subject: [PATCH] Fix memory leak in pyext when Selectable is returned to
 Python (#343)

We have a memory leak in pyext. As result ledd, lddpd, and pmon could use a lot of memory.

I read swig documentation in many places + some source code, but not everything. As I understood from the documentation SWIG needs to have a mechanism to garbage collect objects, which are created inside of C++ method, were given outside, and it is user responsibility to remove the objects. To address such objects with python swig introduced *_OWN flag. So by documentation and sources I read I think we definitely don't need *_OWN flag, because we don't want the selectabl object to be removed, after we receive it from Select.
---
 pyext/Makefile.am      |  2 +-
 pyext/swsscommon.i     |  4 ++--
 tests/test_redis_ut.py | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/pyext/Makefile.am b/pyext/Makefile.am
index ffd70ccd2..14de90f63 100644
--- a/pyext/Makefile.am
+++ b/pyext/Makefile.am
@@ -9,6 +9,6 @@ _swsscommon_la_LDFLAGS = -module
 _swsscommon_la_LIBADD = ../common/libswsscommon.la -lpython$(PYTHON_VERSION)
 
 swsscommon_wrap.cpp: $(SWIG_SOURCES)
-	$(SWIG) -c++ -python -I../common -o $@ $<
+	$(SWIG) -Wall -c++ -python -I../common -o $@ $<
 
 CLEANFILES = swsscommon_wrap.cpp
diff --git a/pyext/swsscommon.i b/pyext/swsscommon.i
index de104d536..094a7ed6a 100644
--- a/pyext/swsscommon.i
+++ b/pyext/swsscommon.i
@@ -42,8 +42,8 @@
         $result = PyList_New(1);
         PyList_SetItem($result, 0, temp);
     }
-    temp = SWIG_NewPointerObj(*$1, SWIGTYPE_p_swss__Selectable, SWIG_POINTER_OWN);
-    PyList_Append($result, temp);
+    temp = SWIG_NewPointerObj(*$1, SWIGTYPE_p_swss__Selectable, 0);
+    SWIG_Python_AppendOutput($result, temp);
 }
 %include "schema.h"
 %include "dbconnector.h"
diff --git a/tests/test_redis_ut.py b/tests/test_redis_ut.py
index ed8741325..81f6fe899 100644
--- a/tests/test_redis_ut.py
+++ b/tests/test_redis_ut.py
@@ -1,4 +1,6 @@
 import time
+from threading import Thread
+from pympler.tracker import SummaryTracker
 from swsscommon import swsscommon
 
 def test_ProducerTable():
@@ -87,3 +89,36 @@ def test_DBConnectorRedisClientName():
     db.setClientName(client_name)
     time.sleep(1)
     assert db.getClientName() == client_name
+
+
+def test_SelectMemoryLeak():
+    N = 50000
+    def table_set(t, state):
+        fvs = swsscommon.FieldValuePairs([("status", state)])
+        t.set("123", fvs)
+
+    def generator_SelectMemoryLeak():
+        app_db = swsscommon.DBConnector("APPL_DB", 0, True)
+        t = swsscommon.Table(app_db, "TABLE")
+        for i in xrange(N/2):
+            table_set(t, "up")
+            table_set(t, "down")
+
+    tracker = SummaryTracker()
+    appl_db = swsscommon.DBConnector("APPL_DB", 0, True)
+    sel = swsscommon.Select()
+    sst = swsscommon.SubscriberStateTable(appl_db, "TABLE")
+    sel.addSelectable(sst)
+    thr = Thread(target=generator_SelectMemoryLeak)
+    thr.daemon = True
+    thr.start()
+    time.sleep(5)
+    for _ in xrange(N):
+        state, c = sel.select(1000)
+    diff = tracker.diff()
+    cases = []
+    for name, count, _ in diff:
+        if count >= N:
+            cases.append("%s - %d objects for %d repeats" % (name, count, N))
+    thr.join()
+    assert not cases