fix: Flush stdout on shutdown matching CPython behavior by majiayu000 · Pull Request #7503 · RustPython/RustPython

majiayu000 · 2026-03-25T04:15:17Z

Summary

RustPython was not flushing stdout on interpreter shutdown, and when stdout flush failed (e.g. broken pipe), it silently ignored the error instead of returning exit code 120 like CPython does.

Changes:

vm_object.rs: flush_std() now returns i32 status (0 = ok, -1 = stdout flush failed). On stdout flush failure, calls run_unraisable() to report the error to stderr. Added file_is_closed() helper to skip flushing already-closed files. Stderr flush errors remain silently ignored (matching CPython).
interpreter.rs: finalize() tracks flush status. If exit_code is 0 and stdout flush failed, returns 120 instead of 0. Non-zero exit codes (e.g. from SystemExit) are preserved.
test_cmd_line.py: Removed @unittest.expectedFailure from test_stdout_flush_at_shutdown.

Test Plan

cargo test --workspace --features threading passes
cargo clippy clean
test_stdout_flush_at_shutdown now passes without expectedFailure

Summary by CodeRabbit

Bug Fixes
- Improved flushing of stdout/stderr during shutdown with better detection of closed streams.
- Exceptions during stdout flushing are now reported/handled instead of being silently ignored.
- Shutdown now returns exit code 120 when an earlier flush failure is detected, aligning exit behavior with expectations.

When stdout flush fails during shutdown, report the error via run_unraisable and exit with code 120 (matching CPython's Py_FinalizeEx). Skip flushing already-closed or None streams. Stderr flush errors remain silently ignored per CPython behavior. Fixes RustPython#5521 Signed-off-by: majiayu000 <1835304752@qq.com>

coderabbitai · 2026-03-25T04:15:25Z

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: 835e2ccf-0c94-4f99-8dec-d996f04b8eba

📥 Commits

Reviewing files that changed from the base of the PR and between 043d00f and c854026.

📒 Files selected for processing (1)

crates/vm/src/vm/interpreter.rs

🚧 Files skipped from review as they are similar to previous changes (1)

crates/vm/src/vm/interpreter.rs

📝 Walkthrough

Walkthrough

Added stdout/stderr flush-status tracking: VirtualMachine::flush_std now returns an i32 status and treats stdout flush exceptions as unraisable; Interpreter::finalize captures that status, retries flush during shutdown, and maps an earlier stdout flush failure to exit code 120 when appropriate.

Changes

Cohort / File(s)	Summary
Interpreter shutdown logic `crates/vm/src/vm/interpreter.rs`	Capture initial `vm.flush_std()` return value, perform a final flush during shutdown, and return exit code `120` (`EXITCODE_FLUSH_FAILURE`) when the earlier stdout flush failed but computed exit code is `0`.
VM I/O flush behavior `crates/vm/src/vm/vm_object.rs`	Changed `VirtualMachine::flush_std` return type from `()` → `i32`; added private `file_is_closed` helper; skip flushing closed/None streams; treat stdout flush exceptions via `run_unraisable` and return `-1` on stdout flush failure (stderr flush skips closed streams but does not report unraisable).

Sequence Diagram(s)

sequenceDiagram
  participant Interpreter
  participant VirtualMachine
  participant StdStream as Stdout/Stderr
  participant Unraisable as run_unraisable

  Interpreter->>VirtualMachine: call flush_std()
  VirtualMachine->>StdStream: check None/closed, call flush()
  alt flush raises exception (stdout)
    VirtualMachine->>Unraisable: run_unraisable(err, None, stdout)
    VirtualMachine-->>Interpreter: return -1 (flush_status)
  else flush succeeds or skipped
    VirtualMachine-->>Interpreter: return 0 (flush_status)
  end

  Interpreter->>VirtualMachine: final flush_std() during shutdown
  VirtualMachine->>StdStream: flush/skip as above
  Interpreter->>Interpreter: if initial flush_status < 0 and exit_code == 0 set exit_code = 120
  Note over Interpreter: exit with computed exit_code

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I thumped my foot, I gave a hop,

I flushed the stream before I stop,
If output splutters, I will sigh,
Exit one-twenty says goodbye,
Hoppity cheers — the logs won't cry 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: Flush stdout on shutdown matching CPython behavior' clearly and specifically summarizes the main change: implementing stdout flushing at shutdown to match CPython.
Linked Issues check	✅ Passed	The PR fully addresses `#5521` requirements: stdout flushing on shutdown is implemented via `flush_std()` returning status, and non-zero exit code 120 is returned on flush failure.
Out of Scope Changes check	✅ Passed	All changes directly address the linked issue: flush status tracking in `vm_object.rs`, exit code handling in `interpreter.rs`, and removing the `@unittest.expectedFailure` decorator—no unrelated modifications detected.
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

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.}

coderabbitai

🧹 Nitpick comments (1)

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

446-448: Optional readability tweak: replace 120 with a named constant.

A small constant (e.g., EXITCODE_FLUSH_STDERR_FAILURE / EXITCODE_FLUSH_FAILURE) would make this CPython-compat mapping easier to maintain.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/vm/interpreter.rs` around lines 446 - 448, Replace the magic
literal 120 with a named constant to improve readability and maintainability:
define a top-level constant (e.g., EXITCODE_FLUSH_FAILURE: i32 = 120) in the
same module and use it in the if branch that currently returns 120 (the code
referencing exit_code and flush_status inside the interpreter logic), e.g.,
change the branch to return EXITCODE_FLUSH_FAILURE instead of the literal;
ensure the constant name clearly indicates its purpose (CPython flush-failure
exit code).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@crates/vm/src/vm/interpreter.rs`:
- Around line 446-448: Replace the magic literal 120 with a named constant to
improve readability and maintainability: define a top-level constant (e.g.,
EXITCODE_FLUSH_FAILURE: i32 = 120) in the same module and use it in the if
branch that currently returns 120 (the code referencing exit_code and
flush_status inside the interpreter logic), e.g., change the branch to return
EXITCODE_FLUSH_FAILURE instead of the literal; ensure the constant name clearly
indicates its purpose (CPython flush-failure exit code).

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 22376891-c5a9-4eed-bd4e-c8ee576083ea

📥 Commits

Reviewing files that changed from the base of the PR and between 211649d and 043d00f.

⛔ Files ignored due to path filters (1)

Lib/test/test_cmd_line.py is excluded by !Lib/**

📒 Files selected for processing (2)

crates/vm/src/vm/interpreter.rs
crates/vm/src/vm/vm_object.rs

github-actions · 2026-03-25T05:04:19Z

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] test: cpython/Lib/test/test_cmd_line.py (TODO: 23)

dependencies:

dependent tests: (no tests depend on cmd_line)

Legend:

[+] path exists in CPython
[x] up-to-date, [ ] outdated

…_FAILURE Address review feedback on PR RustPython#7503: improve readability by extracting the CPython-compat exit code into a named constant. Signed-off-by: majiayu000 <1835304752@qq.com>

arihant2math

This was what I was looking for, thanks. LGTM!

ShaharNaveh

lgtm!
ty, and welcome to the project:)

youknowone

Thank you so much! and welcome to Rustpython project

majiayu000 marked this pull request as ready for review March 25, 2026 04:31

coderabbitai bot reviewed Mar 25, 2026

View reviewed changes

refactor: replace magic number 120 with named constant EXITCODE_FLUSH…

c854026

…_FAILURE Address review feedback on PR RustPython#7503: improve readability by extracting the CPython-compat exit code into a named constant. Signed-off-by: majiayu000 <1835304752@qq.com>

arihant2math approved these changes Mar 25, 2026

View reviewed changes

ShaharNaveh approved these changes Mar 25, 2026

View reviewed changes

youknowone approved these changes Mar 25, 2026

View reviewed changes

youknowone merged commit e1ecb87 into RustPython:main Mar 25, 2026
19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Flush stdout on shutdown matching CPython behavior#7503

fix: Flush stdout on shutdown matching CPython behavior#7503
youknowone merged 2 commits intoRustPython:mainfrom
majiayu000:fix/issue-5521-flush-stdout-on-shutdown

majiayu000 commented Mar 25, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 25, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

github-actions bot commented Mar 25, 2026 •

edited

Loading

Uh oh!

arihant2math left a comment

Uh oh!

ShaharNaveh left a comment

Uh oh!

youknowone left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

majiayu000 commented Mar 25, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Library Dependencies

Uh oh!

arihant2math left a comment

Choose a reason for hiding this comment

Uh oh!

ShaharNaveh left a comment

Choose a reason for hiding this comment

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

majiayu000 commented Mar 25, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 25, 2026 •

edited

Loading

github-actions bot commented Mar 25, 2026 •

edited

Loading