Skip to content

gh-153852: Assert thread list non-empty only after stopping the world (TSAN-0034)#154716

Closed
Vamsi-klu wants to merge 1 commit into
python:mainfrom
Vamsi-klu:fix-gh153852-tsan-0034-finalization-assert
Closed

gh-153852: Assert thread list non-empty only after stopping the world (TSAN-0034)#154716
Vamsi-klu wants to merge 1 commit into
python:mainfrom
Vamsi-klu:fix-gh153852-tsan-0034-finalization-assert

Conversation

@Vamsi-klu

Copy link
Copy Markdown

Summary

Fix TSAN-0034 from the free-threading data-race umbrella gh-153852: a debug-only race on interp->threads.head during finalization.


What the issue is

In handle_thread_shutdown_exception (Python/pylifecycle.c):

assert(interp->threads.head != NULL);  // plain read
_PyEval_StopTheWorld(interp);          // only THEN is the list stable

The comment below correctly says the world-stopped loop does not need locking — but the assert runs before STW. A racing exiting thread in tstate_delete_common can mutate the list under HEAD_LOCK, and TSan reports a data race on the assert's load. Severity is low (debug assert only; NDEBUG strips it), but it is a real race on a free-threaded --with-pydebug build and pollutes TSan signal.


Why I solved it that way

  1. Move the assert after _PyEval_StopTheWorld, do not delete it. The invariant ("finalizing thread is still registered") remains checked; the read becomes race-free by construction.
  2. Do not take HEAD_LOCK for the assert — that would introduce a lock ordered before STW on the finalization path for no benefit and a plausible lock-ordering hazard.
  3. Do not add a regression test that requires TSan — the race is only visible under TSan + pydebug; CI TSan does not always halt_on_error. The fix is a pure ordering change with no release-build codegen impact (assert vanishes under NDEBUG; STW is unconditional in both modes).

How I did it

PyInterpreterState *interp = tstate->interp;
_PyEval_StopTheWorld(interp);
assert(interp->threads.head != NULL);   /* safe: world is stopped */

NEWS under Core and Builtins referencing gh-153852 (TSAN-0034).


Impact

  • Removes a TSan race on free-threaded debug finalization.
  • Zero release-build behavior or performance change.
  • Unblocks cleaner TSan runs for later FT work.

Testing plan

  • Rebuild debug interpreter (GIL configuration available here).
  • ./python -m test test_sys test_threading smoke — OK.
  • Ideal: --disable-gil --with-thread-sanitizer --with-pydebug rebuild + the fusil/TSan repro from the TSAN-0034 gist — not run in this environment (no FT+TSan build). Requesting FT SMEs to confirm on their TSan setup.
  • CI: free-threading / sanitizer jobs.

Everything else


Requested reviewers (subject-matter experts)

Could the following SMEs take a look when convenient (I cannot formally request reviews from this fork account):

Thank you!

… world

Move the threads.head assert in handle_thread_shutdown_exception to after
_PyEval_StopTheWorld so it does not race tstate_delete_common (TSAN-0034).
@python-cla-bot

python-cla-bot Bot commented Jul 26, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: fa67e46142

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Vamsi-klu
Vamsi-klu marked this pull request as draft July 26, 2026 08:01
@Vamsi-klu

Copy link
Copy Markdown
Author

Closing this PR. Thank you for the review attention.

@Vamsi-klu Vamsi-klu closed this Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant