Skip to content

gh-113787: Fix refleaks in test_capi#113816

Merged
nascheme merged 1 commit into
python:mainfrom
neonene:testcapi
Jan 8, 2024
Merged

gh-113787: Fix refleaks in test_capi#113816
nascheme merged 1 commit into
python:mainfrom
neonene:testcapi

Conversation

@neonene

@neonene neonene commented Jan 8, 2024

Copy link
Copy Markdown
Contributor

Since b2566d8, the _testcapi module has specified a non-negative value to PyModuleDef.m_size, which seems to require a bit different refcount management. Every (sub-) interpreter now calls the single phase PyInit__testcapi() with each module state. Previously, the call was only once when running the subinterpreter test case, and repeating Py_INCREF(TestError) n-times in PyInit__testcapi did not affect passing the refleak test for me.

The patch for vectorcall_limited.c follows the way in structmember.c

cc @nascheme @ericsnowcurrently @Eclips4

@Eclips4 Eclips4 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! Thanks.
Refleak tests with this patch:

./python -m test -R 3:3 test_capi
Using random seed: 898642448
0:00:00 load avg: 0.02 Run 1 test sequentially
0:00:00 load avg: 0.02 [1/1] test_capi
beginning 6 repetitions
123456
......
test_capi passed in 33.7 sec

== Tests result: SUCCESS ==

1 test OK.

Total duration: 33.7 sec
Total tests: run=823 skipped=4
Total test files: run=1/1
Result: SUCCESS

@nascheme
nascheme merged commit ace4d7f into python:main Jan 8, 2024
@neonene
neonene deleted the testcapi branch January 8, 2024 16:43
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants