Skip to content

gh-147998: Fix possible memory leak in _pop_preserved (crossinterp.c)#147999

Merged
serhiy-storchaka merged 4 commits intopython:mainfrom
maurycy:maurycy/pop_preserved_shadowing
May 4, 2026
Merged

gh-147998: Fix possible memory leak in _pop_preserved (crossinterp.c)#147999
serhiy-storchaka merged 4 commits intopython:mainfrom
maurycy:maurycy/pop_preserved_shadowing

Conversation

@maurycy
Copy link
Copy Markdown
Contributor

@maurycy maurycy commented Apr 2, 2026 •
edited
Loading

cpython/Python/crossinterp.c

Line 2968 in 7817651

_PyXI_namespace *xidata = _create_sharedns(session->_preserved);

cpython/Python/crossinterp.c

Line 2991 in 7817651

if (xidata != NULL) {

The xidata seems to be shadowed inside the else, and the error: path does not see it, resulting in a (small) memory leak.

It's no longer reproducible on CC=clang CXX=clang++ ./configure --with-pydebug --with-address-sanitizer --without-pymalloc -without-mimalloc build:

[130] 2026-04-02T19:57:40.483100444+0000 maurycy@weiss /home/maurycy/cpython (maurycy/pop_preserved_shadowing) % ./python.exe -m test --single-process test_interpreters -m 'test_callable_requires_frame'

Using random seed: 3999599673
0:00:00 load avg: 1.73 Run 1 test sequentially in a single process
0:00:00 load avg: 1.73 [1/1] test_interpreters
0:00:00 load avg: 1.73 [1/1] test_interpreters passed

== Tests result: SUCCESS ==

1 test OK.

Total duration: 314 ms
Total tests: run=1 (filtered)
Total test files: run=1/1 (filtered)
Result: SUCCESS

@bedevere-app bedevere-app Bot mentioned this pull request Apr 2, 2026
Closed
@serhiy-storchaka serhiy-storchaka self-requested a review April 2, 2026 10:36
serhiy-storchaka
serhiy-storchaka approved these changes Apr 2, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@serhiy-storchaka serhiy-storchaka added the needs backport to 3.14 bugs and security fixes label Apr 2, 2026
@serhiy-storchaka serhiy-storchaka changed the title gh-147998: Fix a memory leak in _pop_preserved gh-147998: Fix possible memory leak in _pop_preserved (crossinterp.c) Apr 2, 2026
@serhiy-storchaka
Copy link
Copy Markdown
Member

serhiy-storchaka commented Apr 2, 2026

Could you please add a NEWS entry?

_pop_preserved() is only used in _PyXI_Exit(), which is used in functions exec(), run_string(), run_func(), call(), and set___main___attrs() of the private module _interpreters (and finally used in some public API). The description should use some general roundabout words to avoid touching internal details.

@maurycy
Copy link
Copy Markdown
Contributor Author

maurycy commented Apr 2, 2026

@serhiy-storchaka 6b415f5

@serhiy-storchaka serhiy-storchaka merged commit 72f29dc into python:main May 4, 2026
54 checks passed
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app Bot commented May 4, 2026

Thanks @maurycy for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented May 4, 2026

GH-149365 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label May 4, 2026
serhiy-storchaka pushed a commit that referenced this pull request May 4, 2026
@miss-islington @maurycy
[3.14] gh-147998: Fix possible memory leak in _pop_preserved (crossin…
fc390bd 
…terp.c) (GH-147999) (GH-149365)

(cherry picked from commit 72f29dc)

Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sergey-miryanov sergey-miryanov sergey-miryanov approved these changes

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@maurycy @serhiy-storchaka @sergey-miryanov