Skip to content

gh-125900: Clean-up logic around immortalization in free-threading#125901

Merged
colesbury merged 5 commits intopython:mainfrom
colesbury:gh-125900-suppress-immortalization
Oct 24, 2024
Merged

gh-125900: Clean-up logic around immortalization in free-threading#125901
colesbury merged 5 commits intopython:mainfrom
colesbury:gh-125900-suppress-immortalization

Conversation

@colesbury
Copy link
Copy Markdown
Contributor

@colesbury colesbury commented Oct 23, 2024

  • Remove @suppress_immortalization decorator
  • Make suppression flag per-thread instead of per-interpreter
  • Suppress immortalization in eval() to avoid refleaks in three tests (test_datetime.test_roundtrip, test_logging.test_config8_ok, and test_random.test_after_fork).
  • frozenset() is constant, but not a singleton. When run multiple times, the test could fail due to constant interning.

* Remove `@suppress_immortalization` decorator
* Make suppression flag per-thread instead of per-interpreter
* Suppress immortalization in `eval()` to avoid refleaks in three tests
  (test_datetime.test_roundtrip, test_logging.test_config8_ok, and
   test_random.test_after_fork).
* frozenset() is constant, but not a singleton. When run multiple times,
  the test could fail due to constant interning.
Comment thread Lib/test/libregrtest/main.py Outdated
Comment thread Lib/test/test_code.py
@@ -141,9 +141,7 @@
ctypes = None
from test.support import (cpython_only,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems that there are too many imports in the current line. Maybe it can be split. See line 146.

This suggestion applies to all changes (maybe I don't like to import too much from the from statement, so it's better to use * directly

(In this way, this () can be removed. Yes, it's still)

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.

I think this is fine. I don't think the PR should do a broader import cleanup -- just undo the changes related @suppress_immortalization.

Comment thread Include/internal/pycore_tstate.h Outdated
Comment thread Lib/test/test_ast/test_ast.py
Comment thread Python/bltinmodule.c Outdated
@colesbury colesbury requested a review from mpage October 24, 2024 17:58
Comment thread Lib/test/test_ast/test_ast.py Outdated
Copy link
Copy Markdown
Contributor

@mpage mpage left a comment

Choose a reason for hiding this comment

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

LGTM!

@colesbury colesbury merged commit 332356b into python:main Oct 24, 2024
@colesbury colesbury deleted the gh-125900-suppress-immortalization branch October 24, 2024 22:10
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
…ing (python#125901)

* Remove `@suppress_immortalization` decorator
* Make suppression flag per-thread instead of per-interpreter
* Suppress immortalization in `eval()` to avoid refleaks in three tests
  (test_datetime.test_roundtrip, test_logging.test_config8_ok, and
   test_random.test_after_fork).
* frozenset() is constant, but not a singleton. When run multiple times,
  the test could fail due to constant interning.
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.

4 participants