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-125900: Clean-up logic around immortalization in free-threading #125901

Merged
merged 5 commits into from
Oct 24, 2024

Conversation

colesbury
Copy link
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.
@@ -141,9 +141,7 @@
ctypes = None
from test.support import (cpython_only,
Copy link
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
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.

Include/internal/pycore_tstate.h Outdated Show resolved Hide resolved
Lib/test/test_ast/test_ast.py Show resolved Hide resolved
Python/bltinmodule.c Outdated Show resolved Hide resolved
@colesbury colesbury requested a review from mpage October 24, 2024 17:58
Copy link
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
46 checks passed
@colesbury colesbury deleted the gh-125900-suppress-immortalization branch October 24, 2024 22:10
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