The Wayback Machine - https://web.archive.org/web/20240110155325/https://github.com/python/cpython/pull/113816
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-113787: Fix refleaks in test_capi #113816

Merged
merged 1 commit into from Jan 8, 2024
Merged

gh-113787: Fix refleaks in test_capi #113816

merged 1 commit into from Jan 8, 2024

Conversation

neonene
Copy link
Contributor

@neonene neonene commented Jan 8, 2024

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

Copy link
Member

@Eclips4 Eclips4 left a comment

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
34 checks passed
@neonene neonene deleted the testcapi branch January 8, 2024 16:43
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.

None yet

3 participants