Skip to content

Commit

Permalink
Fix memory leak in pyext when Selectable is returned to Python (#343)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pavel-shirshov authored May 27, 2020
1 parent 6889c0a commit 5a32636
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 3 deletions.
2 changes: 1 addition & 1 deletion pyext/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions pyext/swsscommon.i
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
35 changes: 35 additions & 0 deletions tests/test_redis_ut.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import time
from threading import Thread
from pympler.tracker import SummaryTracker
from swsscommon import swsscommon

def test_ProducerTable():
Expand Down Expand Up @@ -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

0 comments on commit 5a32636

Please sign in to comment.