Skip to content

Commit 8514386

Browse files
vstinnervstinner
authored
[3.15] gh-157649: Check for reference leak in test_cext (#159296)
* 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. (cherry picked from commit 7952713) * gh-149044: Build test_cext with -Wall -Wextra (#158867) (cherry picked from commit b22a8c9)
1 parent fac4632 commit 8514386

4 files changed

Lines changed: 104 additions & 51 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: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -363,8 +363,6 @@ static PyMethodDef module_methods[] = {
363363
static int
364364
module_exec(PyObject *module)
365365
{
366-
PyObject *result;
367-
368366
#ifdef __STDC_VERSION__
369367
if (PyModule_AddIntMacro(module, __STDC_VERSION__) < 0) {
370368
return -1;
@@ -385,31 +383,8 @@ module_exec(PyObject *module)
385383
return -1;
386384
}
387385
#endif
388-
389-
result = PyObject_CallMethod(module, "test_macros", "");
390-
if (!result) return -1;
391-
Py_DECREF(result);
392-
393-
result = PyObject_CallMethod(module, "test_datetime", "");
394-
if (!result) return -1;
395-
Py_DECREF(result);
396-
397-
result = PyObject_CallMethod(module, "test_unicode", "");
398-
if (!result) return -1;
399-
Py_DECREF(result);
400-
401-
#ifdef __cplusplus
402-
result = PyObject_CallMethod(module, "test_api_casts", "");
403-
if (!result) return -1;
404-
Py_DECREF(result);
405-
#endif
406-
407-
#if defined(__cplusplus) && !defined(Py_TARGET_ABI3T)
408-
result = PyObject_CallMethod(module, "test_virtual_object", "");
409-
if (!result) return -1;
410-
Py_DECREF(result);
411-
#endif
412-
386+
// Ignore "unused argument" warning when none of these macros is defined
387+
(void)module;
413388
return 0;
414389
}
415390

‎Lib/test/test_cext/runtests.py‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
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 (
32+
'__STDC_VERSION__',
33+
'__STRICT_ANSI__',
34+
'__cplusplus',
35+
'_MSVC_LANG',
36+
):
37+
try:
38+
value = getattr(testmod, name)
39+
except AttributeError:
40+
pass
41+
else:
42+
print(f'{name}: {value}')
43+
newline = True
44+
if newline:
45+
print()
46+
47+
if hasattr(sys, 'gettotalrefcount'):
48+
# First run to warm up Python. For example, test_datetime() imports
49+
# the datetime module.
50+
run_tests(testmod, verbose=False)
51+
52+
refcount = array.array('q', [0, 0])
53+
gc.collect()
54+
55+
# Check for reference leak
56+
refcount[0] = sys.gettotalrefcount()
57+
run_tests(testmod)
58+
refcount[1] = sys.gettotalrefcount()
59+
60+
diff = refcount[1] - refcount[0]
61+
if diff >= 1:
62+
raise AssertionError(f'Tests leaked {diff} references')
63+
else:
64+
run_tests(testmod)
65+
66+
67+
if __name__ == "__main__":
68+
main()

‎Lib/test/test_cext/setup.py‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
if not MSVC:
2222
# C compiler flags for GCC and clang
2323
BASE_CFLAGS = [
24+
'-Wall',
25+
'-Wextra',
2426
# The purpose of test_cext extension is to check that building a C
2527
# extension using the Python C API does not emit C compiler warnings.
2628
'-Werror',
@@ -47,7 +49,11 @@
4749
# with the -Werror=declaration-after-statement compiler flag.
4850
'-Werror=declaration-after-statement',
4951
)
50-
INTERNAL_CFLAGS = [*BASE_CFLAGS]
52+
INTERNAL_CFLAGS = [
53+
*BASE_CFLAGS,
54+
# Do not complain about unused parameteres in the internal C API
55+
'-Wno-unused-parameter',
56+
]
5157
else:
5258
# MSVC compiler flags
5359
BASE_CFLAGS = [
@@ -70,14 +76,16 @@
7076

7177
if not MSVC:
7278
# C++ compiler flags for GCC and clang
73-
CPPFLAGS = [
79+
CXXFLAGS = [
80+
'-Wall',
81+
'-Wextra',
7482
# gh-91321: The purpose of _testcppext extension is to check that building
7583
# a C++ extension using the Python C API does not emit C++ compiler
7684
# warnings
7785
'-Werror',
7886
]
7987

80-
CPPFLAGS_PEDANTIC = [
88+
PEDANTIC_CXXFLAGS = [
8189
# Ask for strict(er) compliance with the standard.
8290
# We cannot do this for c++03 unlimited API, since several headers in
8391
# Include/cpython/ use commas at end of `enum` declarations, a C++11
@@ -87,15 +95,20 @@
8795
# We also use `long long`, a C++11 feature we can enable individually.
8896
'-Wno-long-long',
8997
]
98+
INTERNAL_CXXFLAGS = [
99+
# Do not complain about unused parameteres in the internal C API
100+
'-Wno-unused-parameter',
101+
]
90102
else:
91103
# MSVC compiler flags
92-
CPPFLAGS = [
104+
CXXFLAGS = [
93105
# Display warnings level 1 to 4
94106
'/W4',
95107
# Treat all compiler warnings as compiler errors
96108
'/WX',
97109
]
98-
CPPFLAGS_PEDANTIC = []
110+
PEDANTIC_CXXFLAGS = []
111+
INTERNAL_CXXFLAGS = []
99112

100113

101114
def main():
@@ -109,10 +122,13 @@ def main():
109122
libdirs = os.environ.get("CPYTHON_TEST_EXTRA_LIBDIRS", "")
110123
extra_cflags = os.environ.get("CPYTHON_TEST_EXTRA_CFLAGS", "")
111124

125+
if language not in ('C', 'C++'):
126+
raise ValueError(f"invalid language: {language}")
127+
112128
source = SOURCE[language]
113129

114130
if language == 'C++':
115-
flags = list(CPPFLAGS)
131+
flags = list(CXXFLAGS)
116132
else:
117133
if not internal:
118134
flags = list(PUBLIC_CFLAGS)
@@ -127,9 +143,13 @@ def main():
127143
else:
128144
flags.append(f'-std={std}')
129145

130-
if language == 'C++' and (limited or (std != 'c++03') and not internal):
131-
# See CPPFLAGS_PEDANTIC docstring
132-
flags.extend(CPPFLAGS_PEDANTIC)
146+
if language == 'C++':
147+
if limited or (std != 'c++03') and not internal:
148+
# See PEDANTIC_CXXFLAGS comment
149+
flags.extend(PEDANTIC_CXXFLAGS)
150+
elif internal:
151+
flags.extend(INTERNAL_CXXFLAGS)
152+
133153

134154
# gh-105776: When "gcc -std=11" is used as the C++ compiler, -std=c11
135155
# option emits a C++ compiler warning. Remove "-std11" option from the

0 commit comments

Comments
 (0)