Skip to content

Commit 7952713

Browse files
authored
gh-157649: Check for reference leak in test_cext (#158227)
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 8122ff4 commit 7952713

3 files changed

Lines changed: 82 additions & 42 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: 14 additions & 27 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,13 @@ 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);
376+
#ifdef _MSVC_LANG
377+
if (PyModule_AddIntMacro(module, _MSVC_LANG) < 0) {
378+
return -1;
379+
}
394380
#endif
395-
381+
// Ignore "unused argument" warning when none of these macros is defined
382+
(void)module;
396383
return 0;
397384
}
398385

‎Lib/test/test_cext/runtests.py‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
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+
print(f"{name}()")
13+
func()
14+
15+
print("add()")
16+
if testmod.add(11, 23) != 34:
17+
raise AssertionError("add() failed badly")
18+
19+
print(flush=True)
20+
21+
22+
def main():
23+
if len(sys.argv) < 2:
24+
print("usage: python runtests.py TEST_MODULE_NAME")
25+
sys.exit(1)
26+
module_name = sys.argv[1]
27+
28+
testmod = importlib.import_module(module_name)
29+
30+
newline = False
31+
for name in ('__STDC_VERSION__', '__cplusplus', '_MSVC_LANG'):
32+
try:
33+
value = getattr(testmod, name)
34+
except AttributeError:
35+
pass
36+
else:
37+
print(f'{name}: {value}')
38+
newline = True
39+
if newline:
40+
print()
41+
42+
if hasattr(sys, 'gettotalrefcount'):
43+
# First run to warm up Python. For example, test_datetime() imports
44+
# the datetime module.
45+
run_tests(testmod, verbose=False)
46+
47+
refcount = array.array('q', [0, 0])
48+
gc.collect()
49+
50+
# Check for reference leak
51+
refcount[0] = sys.gettotalrefcount()
52+
run_tests(testmod)
53+
refcount[1] = sys.gettotalrefcount()
54+
55+
diff = refcount[1] - refcount[0]
56+
if diff >= 1:
57+
raise AssertionError(f'Tests leaked {diff} references')
58+
else:
59+
run_tests(testmod)
60+
61+
62+
if __name__ == "__main__":
63+
main()

0 commit comments

Comments
 (0)