Align shutdown flush and unraisable reporting with CPython - #9810

This comment shows the latest Codex review activity on this pull request.

The changes add retained interactive input, timed mutex acquisition, shutdown-specific I/O locking, and revised FileIO error conversion. They also centralize unraisable exception output and change when standard streams are flushed during interpreter finalization.

Changes: Runtime I/O and shutdown

Layer / File(s)	Summary
Timed mutex acquisition crates/common/src/lock/thread_mutex.rs	ThreadMutex adds wrapped timed acquisition. It distinguishes current-thread ownership from contention and tests both cases, plus acquisition after unlocking.
Retained interactive input crates/host_env/src/readline.rs, crates/vm/src/stdlib/builtins.rs, extra_tests/snippets/stdlib_input_readline.py	Interactive input() uses a retained reader keyed by terminal identity. Reentrant reads return an error. PTY tests cover worker-thread input, terminal changes, and nested input from an audit hook.
Shutdown locks and FileIO errors crates/vm/src/stdlib/_io.rs	Buffered I/O locks use a one-second timed acquisition during finalization. Reentrant acquisition raises a runtime error; contention by another thread writes a diagnostic and aborts. FileIO read, write, and close errors no longer attach the stream name as the filename.
Shared unraisable reporting crates/vm/src/exceptions.rs, crates/vm/src/object/core.rs, crates/vm/src/stdlib/sys.rs, crates/vm/src/vm/mod.rs	The VM and sys module use a shared unraisable writer. It tries Python traceback formatting, then uses native fallback output. Finalizer errors pass None as the object argument.
Shutdown flush order and status crates/vm/src/vm/interpreter.rs, crates/vm/src/vm/vm_object.rs	Finalization flushes streams after exit callbacks and subinterpreter cleanup. Flush failures set the status to 120 in the interpreter exit paths. flush_std reads streams from sys.__dict__ and reports stdout failures as unraisable.

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant BuiltinsInput
  participant input_readline
  participant InputReader
  participant Readline
  BuiltinsInput->>input_readline: prompt
  input_readline->>InputReader: check identity and reuse or replace reader
  InputReader->>Readline: read prompt
  Readline-->>input_readline: ReadlineResult
  input_readline-->>BuiltinsInput: ReadlineResult

Loading

Possibly related PRs

• RustPython/RustPython#9690: The PR continues the related work on unraisable reporting, shutdown flushing, I/O locking, and retained input.

Suggested reviewers: shaharnaveh, youknowdot, coolreader18

Merge Risk: Low · up to a0ee4

Shutdown and input behavior look functionally sound. One new lock helper has a contract that could be misused by future callers; tightening it before merge is advisable. The remaining fork-time input edge case is narrow.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Out of Scope Changes check		The input() changes do not implement any requirement of #9690. The issue covers shutdown flush, unraisable reporting, and the stdout/stderr exit status only. The following changes are out of scope: -…	Remove the input() reader, readline, PTY test, and try_lock_for_wrapped changes from this PR, or move them to a separate PR with its own linked issue. Restore the test_cmd_line.test_stdout_flush_at_shutdown expected-failure marker so th…

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main theme of the changeset, which is aligning shutdown stdout/stderr flush behavior and unraisable exception reporting with CPython. The raw summary confirms changes to interprete…
Linked Issues check		Directly linked issue #9690 has only coding requirements. Each requirement maps to changed code in the whole-PR summary: - Shared fallible default writer: stdlib::sys::write_unraisable in `crates/v…

Full details: Title check

Explanation

The title names the main theme of the changeset, which is aligning shutdown stdout/stderr flush behavior and unraisable exception reporting with CPython. The raw summary confirms changes to interpreter finalization order, flush_std, the unraisable hook path in vm/mod.rs and sys.rs, and the exit status 120 rule. The title omits the secondary changes (input() readline reader, thread lock timeouts, FileIO filename removal), but the PR description says those are follow-ups, so this is a partial omission rather than a mismatch. The title is concise and specific enough for a reviewer scanning history.

Full details: Linked Issues check

Explanation

Directly linked issue #9690 has only coding requirements. Each requirement maps to changed code in the whole-PR summary:

• Shared fallible default writer: stdlib::sys::write_unraisable in crates/vm/src/stdlib/sys.rs serves both __unraisablehook__ and the VM fallback in crates/vm/src/vm/mod.rs.
• Explicit-file traceback formatting with native fallback: VirtualMachine::try_display_exception_to in crates/vm/src/exceptions.rs passes the caller's file to the Python formatter. The native fallback covers <object repr() failed> and <exception str() failed>.
• Direct sys-dict access: crates/vm/src/vm/vm_object.rs reads stdout and stderr from sys.__dict__. crates/vm/src/vm/mod.rs reads unraisablehook from the sys module dictionary.
• Audit order and user hook during finalization: the PR description states both. The summary shows the hook lookup change but does not show the finalization path explicitly. This point rests partly on the PR description.
• Flush placement: crates/vm/src/vm/interpreter.rs moves the initial stdout/stderr flush after exit callbacks and subinterpreter cleanup.
• Stdout failure message and stderr silent counting: vm_object.rs reports stdout failures as unraisable with a message naming sys.stdout. Stderr failures set status -1 without reporting.
• Status 120: interpreter.rs overrides any requested exit code with 120 when the shutdown flush fails.

The lock-timeout and fatal fd 2 diagnostic in _io.rs are extra shutdown hardening and do not conflict with the issue. Automated tests cited in the PR description (test_cmd_line, test_sys, test_io) were not independently run here.

Full details: Out of Scope Changes check

Explanation

The input() changes do not implement any requirement of #9690. The issue covers shutdown flush, unraisable reporting, and the stdout/stderr exit status only. The following changes are out of scope:

• crates/host_env/src/readline.rs: adds a process-wide retained reader, TerminalIdentity, ReentrantReadline, and input_readline.
• crates/vm/src/stdlib/builtins.rs: switches input() to input_readline and adds the can't re-enter readline error.
• extra_tests/snippets/stdlib_input_readline.py: adds a POSIX PTY harness for input() behavior.
• crates/common/src/lock/thread_mutex.rs: adds try_lock_for_wrapped, which exists to support the readline or lock work and is not required by #9690.

The PR description also states that these changes go beyond the issue's stated scope. The FileIO filename change in _io.rs is connected to the shutdown stdout message and is not counted here.

Separately, the PR description says it removes the test_cmd_line.test_stdout_flush_at_shutdown expected-failure marker. The issue says no Lib or test files are edited and the marker is unchanged. Lib/test/test_cmd_line.py is in ignored_files, so its contents were not reviewed. This conflict cannot be resolved from the available evidence.

Resolution

Remove the input() reader, readline, PTY test, and try_lock_for_wrapped changes from this PR, or move them to a separate PR with its own linked issue. Restore the test_cmd_line.test_stdout_flush_at_shutdown expected-failure marker so this PR does not edit Lib test files, as #9690 requires. Keep the shutdown flush, unraisable reporting, FileIO error-filename, and deallocator-message changes.

Generate unit tests (beta)

• Create a new PR

• X
• Mastodon
• Reddit
• LinkedIn