Skip to content

Commit 5a32636

Browse files
Fix memory leak in pyext when Selectable is returned to Python (sonic-net#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.
1 parent 6889c0a commit 5a32636

3 files changed

Lines changed: 38 additions & 3 deletions

File tree

pyext/Makefile.am

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@ _swsscommon_la_LDFLAGS = -module
99
_swsscommon_la_LIBADD = ../common/libswsscommon.la -lpython$(PYTHON_VERSION)
1010

1111
swsscommon_wrap.cpp: $(SWIG_SOURCES)
12-
$(SWIG) -c++ -python -I../common -o $@ $<
12+
$(SWIG) -Wall -c++ -python -I../common -o $@ $<
1313

1414
CLEANFILES = swsscommon_wrap.cpp

pyext/swsscommon.i

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@
4242
$result = PyList_New(1);
4343
PyList_SetItem($result, 0, temp);
4444
}
45-
temp = SWIG_NewPointerObj(*$1, SWIGTYPE_p_swss__Selectable, SWIG_POINTER_OWN);
46-
PyList_Append($result, temp);
45+
temp = SWIG_NewPointerObj(*$1, SWIGTYPE_p_swss__Selectable, 0);
46+
SWIG_Python_AppendOutput($result, temp);
4747
}
4848
%include "schema.h"
4949
%include "dbconnector.h"

tests/test_redis_ut.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import time
2+
from threading import Thread
3+
from pympler.tracker import SummaryTracker
24
from swsscommon import swsscommon
35

46
def test_ProducerTable():
@@ -87,3 +89,36 @@ def test_DBConnectorRedisClientName():
8789
db.setClientName(client_name)
8890
time.sleep(1)
8991
assert db.getClientName() == client_name
92+
93+
94+
def test_SelectMemoryLeak():
95+
N = 50000
96+
def table_set(t, state):
97+
fvs = swsscommon.FieldValuePairs([("status", state)])
98+
t.set("123", fvs)
99+
100+
def generator_SelectMemoryLeak():
101+
app_db = swsscommon.DBConnector("APPL_DB", 0, True)
102+
t = swsscommon.Table(app_db, "TABLE")
103+
for i in xrange(N/2):
104+
table_set(t, "up")
105+
table_set(t, "down")
106+
107+
tracker = SummaryTracker()
108+
appl_db = swsscommon.DBConnector("APPL_DB", 0, True)
109+
sel = swsscommon.Select()
110+
sst = swsscommon.SubscriberStateTable(appl_db, "TABLE")
111+
sel.addSelectable(sst)
112+
thr = Thread(target=generator_SelectMemoryLeak)
113+
thr.daemon = True
114+
thr.start()
115+
time.sleep(5)
116+
for _ in xrange(N):
117+
state, c = sel.select(1000)
118+
diff = tracker.diff()
119+
cases = []
120+
for name, count, _ in diff:
121+
if count >= N:
122+
cases.append("%s - %d objects for %d repeats" % (name, count, N))
123+
thr.join()
124+
assert not cases

0 commit comments

Comments
 (0)