FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Stop non-main threads during interpreter finalization by youknowone · Pull Request #7349 · RustPython/RustPython · GitHub

/ RustPython Public

Stop non-main threads during interpreter finalization - #7349

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:thread-safety
Mar 5, 2026
ConversationCommits (1)ChecksFiles changed
Merged

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

Conversation

youknowone commented Mar 4, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

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.

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 Bot commented Mar 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 marked this pull request as ready for review March 4, 2026 16:43

chatgpt-codex-connector Bot commented Mar 4, 2026

Copy link
Copy Markdown

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 merged commit 62766fd into RustPython:main Mar 5, 2026
23 of 24 checks passed
youknowone deleted the thread-safety branch March 5, 2026 01:21
coderabbitai Bot mentioned this pull request Mar 6, 2026
Merged
youknowone added a commit to youknowone/RustPython that referenced this pull request Mar 22, 2026
Stop non-main threads during interpreter finalization (RustPython#7349)
e84cd95 
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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

Back | FazBrowse Home | New Git URL