Skip to content

Commit

Permalink
Fix PyUnknownFields memory leak (#7928)
Browse files Browse the repository at this point in the history
Properly release internal data structure on deallocation.
Fix #7301
  • Loading branch information
agrenott committed Nov 6, 2020
1 parent 0bb6dd8 commit 5f7f389
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 0 deletions.
24 changes: 24 additions & 0 deletions python/google/protobuf/internal/unknown_fields_test.py
Expand Up @@ -39,6 +39,12 @@
import unittest2 as unittest #PY26
except ImportError:
import unittest
import sys
try:
import tracemalloc
except ImportError:
# Requires python 3.4+
pass
from google.protobuf import map_unittest_pb2
from google.protobuf import unittest_mset_pb2
from google.protobuf import unittest_pb2
Expand Down Expand Up @@ -312,6 +318,24 @@ def testClear(self):
self.assertIn('UnknownFields does not exist.',
str(context.exception))

@unittest.skipIf((sys.version_info.major, sys.version_info.minor) < (3, 4),
'tracemalloc requires python 3.4+')
def testUnknownFieldsNoMemoryLeak(self):
# Call to UnknownFields must not leak memory
nb_leaks = 1234
def leaking_function():
for _ in range(nb_leaks):
self.empty_message.UnknownFields()
tracemalloc.start()
snapshot1 = tracemalloc.take_snapshot()
leaking_function()
snapshot2 = tracemalloc.take_snapshot()
top_stats = snapshot2.compare_to(snapshot1, 'lineno')
tracemalloc.stop()
# There's no easy way to look for a precise leak source.
# Rely on a "marker" count value while checking allocated memory.
self.assertEqual([], [x for x in top_stats if x.count_diff == nb_leaks])

def testSubUnknownFields(self):
message = unittest_pb2.TestAllTypes()
message.optionalgroup.a = 123
Expand Down
1 change: 1 addition & 0 deletions python/google/protobuf/pyext/unknown_fields.cc
Expand Up @@ -142,6 +142,7 @@ static void Dealloc(PyObject* pself) {
}
Py_CLEAR(self->parent);
self->~PyUnknownFields();
Py_TYPE(pself)->tp_free(pself);
}

static PySequenceMethods SqMethods = {
Expand Down

0 comments on commit 5f7f389

Please sign in to comment.