Skip to content

Stop non-main threads during interpreter finalization#7349

Merged
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:thread-safety
Mar 5, 2026
Merged

Stop non-main threads during interpreter finalization#7349
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:thread-safety

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Mar 4, 2026 •
edited by coderabbitai bot
Loading

Raise SystemExit in check_signals() for non-main threads once the finalizing flag is set. Without GIL, this serves as the checkpoint where daemon threads detect shutdown, analogous to CPython's _PyThreadState_MustExit in take_gil.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error handling in multi-threaded configurations to properly manage edge cases during application shutdown.

@youknowone
Stop non-main threads during interpreter finalization
cb3f803 
Raise SystemExit in check_signals() for non-main threads
once the finalizing flag is set. Without GIL, this serves
as the checkpoint where daemon threads detect shutdown,
analogous to CPython's _PyThreadState_MustExit in take_gil.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 4, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 6ae3b468-6d9a-42bb-bfa9-596ee04ea8de

📥 Commits

Reviewing files that changed from the base of the PR and between 5c29074 and cb3f803.

📒 Files selected for processing (1)
  • crates/vm/src/vm/mod.rs
📝 Walkthrough

Walkthrough

A runtime guard is added to the check_signals function that detects when finalization has started and the current thread is not the main thread (only when the "threading" feature is enabled and not on wasm32 targets), returning a SystemExit exception to halt execution in this scenario.

Changes

Cohort / File(s) Summary
Finalization Guard
crates/vm/src/vm/mod.rs		 Adds a conditional check in check_signals that returns a SystemExit exception when finalization has begun on a non-main thread, gated behind the threading feature and platform-specific exclusions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • RustPython/RustPython#6626: Modifies VM finalization control-flow by reordering finalizing/atexit/thread shutdown and adding finalization checks in related shutdown paths.

Poem

🐰 A guard stands watch at signal's gate,
"Hold! Finalization's underway,"
No non-main threads shall pass this fate,
SystemExit saves the day! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a runtime guard that stops non-main threads during interpreter finalization by raising SystemExit in check_signals().
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone youknowone marked this pull request as ready for review March 4, 2026 16:43
@chatgpt-codex-connector
Copy link

chatgpt-codex-connector bot commented Mar 4, 2026

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@youknowone youknowone merged commit 62766fd into RustPython:main Mar 5, 2026
23 of 24 checks passed
@youknowone youknowone deleted the thread-safety branch March 5, 2026 01:21
@coderabbitai coderabbitai bot mentioned this pull request Mar 6, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone