Skip to content

Commit 71bcc9d

Browse files
committed
gh-157649: Check for reference leak in test_cext
test_cext now discovers and calls all "test" functions. On a debug build, it also checks for reference leaks. Fix a reference leak in test_virtual_object(): use a dealloc function which calls Py_DECREF() on the type, and trigger an explicit garbage collection.
1 parent 499dff1 commit 71bcc9d

3 files changed

Lines changed: 66 additions & 43 deletions

File tree

‎Lib/test/test_cext/__init__.py‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
os.path.join(SOURCE_DIR, 'extension.cpp'),
2727
os.path.join(SOURCE_DIR, 'setup.py'),
2828
]
29+
RUNTESTS_PY = os.path.join(SOURCE_DIR, 'runtests.py')
2930
MSVC = support.MS_WINDOWS
3031

3132

@@ -99,21 +100,10 @@ def run_cmd(operation, cmd):
99100
cmd.append('-v')
100101
run_cmd('Install', cmd)
101102

102-
# Do a reference run. Until we test that running python
103-
# doesn't leak references (gh-94755), run it so one can manually check
104-
# -X showrefcount results against this baseline.
105-
cmd = [python_exe,
106-
'-X', 'dev',
107-
'-X', 'showrefcount',
108-
'-c', 'pass']
109-
run_cmd('Reference run', cmd)
110-
111-
# Import the C/C++ extension
112-
cmd = [python_exe,
113-
'-X', 'dev',
114-
'-X', 'showrefcount',
115-
'-c', f"import {extension_name}"]
116-
run_cmd('Import', cmd)
103+
# Import the extension module and run tests.
104+
# On a debug build, check also for reference leaks.
105+
cmd = [python_exe, '-X', 'dev', RUNTESTS_PY, extension_name]
106+
run_cmd('Tests', cmd)
117107

118108

119109
class TestPublicC(BaseTests, unittest.TestCase):

‎Lib/test/test_cext/extension.c‎

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,11 @@ class VirtualPyObject : public PyObject {
264264
public:
265265
VirtualPyObject();
266266
virtual ~VirtualPyObject() {
267+
PyTypeObject *type = Py_TYPE(this);
267268
delete [] internal_data;
268269
--instance_count;
270+
// Do not call type->tp_free(this), C++ manages the memory
271+
Py_DECREF(type);
269272
}
270273
virtual void set_internal_data() {
271274
internal_data[0] = 1;
@@ -295,7 +298,7 @@ _Py_COMP_DIAG_PUSH
295298
#endif
296299

297300
PyType_Slot VirtualPyObject_Slots[] = {
298-
{Py_tp_free, (void*)VirtualPyObject::dealloc},
301+
{Py_tp_dealloc, (void*)VirtualPyObject::dealloc},
299302
{0, _Py_NULL},
300303
};
301304

@@ -333,6 +336,10 @@ test_virtual_object(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args))
333336
"instance_count should be 0, got %d",
334337
VirtualPyObject::instance_count);
335338
}
339+
340+
// Force a garbage collection to delete the temporary heap type
341+
// used by this test
342+
PyGC_Collect();
336343
Py_RETURN_NONE;
337344
}
338345
#endif // __cplusplus && !Py_TARGET_ABI3T
@@ -356,8 +363,6 @@ static PyMethodDef module_methods[] = {
356363
static int
357364
module_exec(PyObject *module)
358365
{
359-
PyObject *result;
360-
361366
#ifdef __STDC_VERSION__
362367
if (PyModule_AddIntMacro(module, __STDC_VERSION__) < 0) {
363368
return -1;
@@ -368,31 +373,6 @@ module_exec(PyObject *module)
368373
return -1;
369374
}
370375
#endif
371-
372-
result = PyObject_CallMethod(module, "test_macros", "");
373-
if (!result) return -1;
374-
Py_DECREF(result);
375-
376-
result = PyObject_CallMethod(module, "test_datetime", "");
377-
if (!result) return -1;
378-
Py_DECREF(result);
379-
380-
result = PyObject_CallMethod(module, "test_unicode", "");
381-
if (!result) return -1;
382-
Py_DECREF(result);
383-
384-
#ifdef __cplusplus
385-
result = PyObject_CallMethod(module, "test_api_casts", "");
386-
if (!result) return -1;
387-
Py_DECREF(result);
388-
#endif
389-
390-
#if defined(__cplusplus) && !defined(Py_TARGET_ABI3T)
391-
result = PyObject_CallMethod(module, "test_virtual_object", "");
392-
if (!result) return -1;
393-
Py_DECREF(result);
394-
#endif
395-
396376
return 0;
397377
}
398378

‎Lib/test/test_cext/runtests.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import array
2+
import gc
3+
import importlib
4+
import sys
5+
6+
7+
def run_tests(testmod, verbose=True):
8+
for name in dir(testmod):
9+
if not name.startswith('test'):
10+
continue
11+
func = getattr(testmod, name)
12+
if verbose:
13+
print(f"{name}()")
14+
func()
15+
16+
print("add()")
17+
if testmod.add(11, 23) != 34:
18+
raise AssertionError("add() failed badly")
19+
20+
if verbose:
21+
print(flush=True)
22+
23+
24+
def main():
25+
if len(sys.argv) < 2:
26+
print("usage: python runtests.py TEST_MODULE_NAME")
27+
sys.exit(1)
28+
module_name = sys.argv[1]
29+
30+
testmod = importlib.import_module(module_name)
31+
32+
if hasattr(sys, 'gettotalrefcount'):
33+
# First run to warm up Python. For example, test_datetime() imports
34+
# the datetime module.
35+
run_tests(testmod, verbose=False)
36+
37+
refcount = array.array('q', [0, 0])
38+
gc.collect()
39+
40+
# Check for reference leak
41+
refcount[0] = sys.gettotalrefcount()
42+
run_tests(testmod)
43+
refcount[1] = sys.gettotalrefcount()
44+
45+
diff = refcount[1] - refcount[0]
46+
if diff >= 1:
47+
raise AssertionError(f'Tests leaked {diff} references')
48+
else:
49+
run_tests(testmod)
50+
51+
52+
if __name__ == "__main__":
53+
main()

0 commit comments

Comments
 (0)