Add PythonFinallizationError and upgrade multiprocessing by youknowone · Pull Request #6592 · RustPython/RustPython

youknowone · 2025-12-30T05:18:38Z

Summary by CodeRabbit

Bug Fixes

Improved error handling for subprocess operations attempted during interpreter shutdown with clearer error messaging.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-30T05:18:44Z

📝 Walkthrough

Walkthrough

This change introduces a new PythonFinalizationError exception type to the VM's exception system and uses it to prevent preexec_fn execution during interpreter shutdown in the POSIX subprocess module. The exception type is registered as a builtin exception subclassing PyRuntimeError, with a corresponding factory method added to VirtualMachine.

Changes

Cohort / File(s)	Summary
Exception infrastructure `crates/vm/src/exceptions.rs`, `crates/vm/src/vm/vm_new.rs`	Introduces new `PyPythonFinalizationError` exception type (subclass of `PyRuntimeError`) with field addition to `ExceptionZoo`, initialization in `ExceptionZoo::init`, and exception extension registration. Adds corresponding `new_python_finalization_error()` factory method to `VirtualMachine` via `define_exception_fn!` macro.
POSIX subprocess guard `crates/stdlib/src/posixsubprocess.rs`	Adds early-return guard in `fork_exec()` that detects when `preexec_fn` is provided during interpreter finalization and raises `PythonFinalizationError` with message "preexec_fn not supported at interpreter shutdown" before argument processing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

impl preexec_fn #6479: Related changes to preexec_fn handling in fork_exec() and VM threading through execution context
Convert new_X_error to use a macro #5814: Introduces the define_exception_fn! macro pattern for generating exception constructor methods on VirtualMachine

Poem

🐰 A finalization guard hops into view,
Preexec_fn shutdown—now blocked on cue,
New exception type, cleanly designed,
Error handling flow, precisely aligned! 🎯

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.
Title check	❓ Inconclusive	The title mentions 'PythonFinallizationError' (with a typo: 'Finalli' instead of 'Finali') and references 'upgrade multiprocessing', but the summary shows changes only to posixsubprocess.rs, exceptions.rs, and vm_new.rs related to adding the error type and a guard in fork_exec. The 'upgrade multiprocessing' aspect is unclear and not substantiated by the provided summaries.	Clarify what 'upgrade multiprocessing' entails in the PR description or update the title to accurately reflect only the PythonFinalizationError additions. Also fix the spelling: 'PythonFinalizationError' not 'PythonFinallizationError'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e1fc93 and 406376c.

⛔ Files ignored due to path filters (3)

Lib/multiprocessing/connection.py is excluded by !Lib/**
Lib/multiprocessing/resource_tracker.py is excluded by !Lib/**
Lib/multiprocessing/util.py is excluded by !Lib/**

📒 Files selected for processing (3)

crates/stdlib/src/posixsubprocess.rs
crates/vm/src/exceptions.rs
crates/vm/src/vm/vm_new.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/stdlib/src/posixsubprocess.rs
crates/vm/src/vm/vm_new.rs
crates/vm/src/exceptions.rs

🧠 Learnings (1)

📚 Learning: 2025-12-09T08:46:58.660Z

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6358
File: crates/vm/src/exception_group.rs:173-185
Timestamp: 2025-12-09T08:46:58.660Z
Learning: In crates/vm/src/exception_group.rs, the derive() method intentionally always creates a BaseExceptionGroup instance rather than preserving the original exception class type. This is a deliberate design decision that differs from CPython's behavior.

Applied to files:

crates/vm/src/vm/vm_new.rs
crates/vm/src/exceptions.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Run snippets and cpython tests (windows-latest)

🔇 Additional comments (7)

crates/stdlib/src/posixsubprocess.rs (1)

36-46: LGTM! Guard correctly prevents preexec_fn execution during shutdown.

The early check with Ordering::Acquire is appropriate for reading a flag potentially set by another thread, and the placement before resource allocation avoids unnecessary work when the error will be raised.

crates/vm/src/vm/vm_new.rs (1)

603-603: LGTM! Follows established pattern for exception factory methods.

The new constructor is consistent with existing exception factory methods like new_runtime_error and new_recursion_error, using the same define_exception_fn! macro pattern.

crates/vm/src/exceptions.rs (5)

496-496: Field declaration follows established ordering.

Placed correctly after recursion_error within the RuntimeError subclass group.

808-808: Initialization follows established pattern.

The init_builtin_type() call is correctly placed after recursion_error initialization, maintaining consistency with the ExceptionZoo field ordering.

884-884: Struct field assignment is consistent.

Correctly placed in the same relative position as the field declaration and initialization.

998-1002: Extension registration follows established pattern.

The extend_exception! macro call correctly registers PyPythonFinalizationError without any additional attributes, which is appropriate for this simple exception type.

2122-2125: LGTM! Exception type correctly subclasses PyRuntimeError.

The implementation follows the established pattern for simple exception types, matching the structure of PyNotImplementedError and PyRecursionError. All registration steps are complete and consistent across the ExceptionZoo struct, initialization, assignment, and macro extension.

Note: PythonFinalizationError was introduced in Python 3.13 (not 3.12) as a subclass of RuntimeError.

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 added 3 commits December 30, 2025 14:12

PythonFinallizationError

8dd6655

fix posixprocess

f306915

upgrade multiprocessing from 3.13.11

406376c

youknowone marked this pull request as ready for review December 30, 2025 05:43

youknowone merged commit 6ed0b4f into RustPython:main Dec 30, 2025
13 checks passed

youknowone deleted the multiprocessing branch December 30, 2025 06:15

coderabbitai bot mentioned this pull request Jan 17, 2026

fix finalizing and atexit timing #6626

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PythonFinallizationError and upgrade multiprocessing#6592

Add PythonFinallizationError and upgrade multiprocessing#6592
youknowone merged 3 commits intoRustPython:mainfrom
youknowone:multiprocessing

youknowone commented Dec 30, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 30, 2025 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

youknowone commented Dec 30, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Bug Fixes

Uh oh!

coderabbitai bot commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Pre-merge checks and finishing touches

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Dec 30, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 30, 2025 •

edited

Loading