Skip to content

gh-159538: Fix deep chain leak in test_deeply_dotted_name - #159576

Merged
vstinner merged 4 commits into
python:mainfrom
brittanyrey:fix-lazy-deep-chain-leak
Oct 17, 2026
Merged

vstinner merged 4 commits into
python:mainfrom
brittanyrey:fix-lazy-deep-chain-leak

Conversation

@brittanyrey

@brittanyrey brittanyrey commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

test_deeply_dotted_name() builds a 10,000-deep chain of lazy import placeholders in a thread with a 256 KiB C stack. This was causing a refleak.

Address it by explicitly adding a cleanup step to clear the import chain.

test_deeply_dotted_name() builds a 10,000-deep chain of lazy import
placeholders in a thread with a 256 KiB C stack.  Freeing the chain
there defers most deallocations to the trashcan, which only drains them
once the stack margin recovers.  With a stack that small the margin
never does, so the deferred objects were leaked when the thread exited,
which -R reports as a reference leak on platforms with larger frames
such as PPC64LE.

Clear the namespace in a cleanup, which runs on the main thread.
""")
assert_python_ok("-c", code)

@support.run_with_limited_c_stack(depth=10_000, size=256 * 1024)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This would also work as a fix:

Suggested change
@support.run_with_limited_c_stack(depth=10_000, size=512 * 1024)

@vstinner vstinner 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.

LGTM. I confirm that this change fix ./python -m test -R 3:3 test_lazy_import -m test_deeply_dotted_name on PPC64LE CentOS9.

@vstinner
vstinner merged commit c84d4b5 into python:main Oct 17, 2026
58 checks passed
@vstinner

Copy link
Copy Markdown
Member

Merged, thanks for the fix.

@vstinner

Copy link
Copy Markdown
Member

By the way, I confirm that the following change also fix the issue on PPC64LE CentOS9:

diff --git a/Lib/test/test_lazy_import/__init__.py b/Lib/test/test_lazy_import/__init__.py
index 31006f8..84ec28e 100644
--- a/Lib/test/test_lazy_import/__init__.py
+++ b/Lib/test/test_lazy_import/__init__.py
@@ -308,7 +308,7 @@ def test_lazy_import_type_attribute_error_message(self):
         """)
         assert_python_ok("-c", code)
 
-    @support.run_with_limited_c_stack(depth=10_000, size=256 * 1024)
+    @support.run_with_limited_c_stack(depth=10_000, size=512 * 1024)
     def test_deeply_dotted_name(self):
         """A long dotted name must not overflow the C stack."""
         m = types.SimpleNamespace()

But this change seems to be less reliable to me, since it relies on specific constants. I prefer the generic self.addCleanup(ns.clear).

@vstinner vstinner added the needs backport to 3.15 bugs and security fixes label Oct 17, 2026
@miss-islington-app

Copy link
Copy Markdown

Thanks @brittanyrey for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Oct 17, 2026

Copy link
Copy Markdown

GH-159609 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Oct 17, 2026
@vstinner

Copy link
Copy Markdown
Member

Oh, Python 3.15 is also affected, see test_lazy_import failure on PPC64LE Fedora Stable Refleaks 3.15: https://buildbot.python.org/#/builders/2087/builds/242.

vstinner pushed a commit that referenced this pull request Oct 17, 2026
…159576) (#159609)

gh-159538: Fix deep chain leak in test_deeply_dotted_name (GH-159576)

test_deeply_dotted_name() builds a 10,000-deep chain of lazy import
placeholders in a thread with a 256 KiB C stack.  Freeing the chain
there defers most deallocations to the trashcan, which only drains them
once the stack margin recovers.  With a stack that small the margin
never does, so the deferred objects were leaked when the thread exited,
which -R reports as a reference leak on platforms with larger frames
such as PPC64LE.

Clear the namespace in a cleanup, which runs on the main thread.
(cherry picked from commit c84d4b5)

Co-authored-by: Brittany Reynoso <breynoso@meta.com>
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.

2 participants