From 721d5737b997baae21ba0fd375847edb7ad9aee4 Mon Sep 17 00:00:00 2001 From: dot Date: Wed, 4 Nov 2026 13:37:36 +0900 Subject: [PATCH] Align shutdown flush and unraisable reporting with CPython Share the fallible default writer, retain the selected output stream across callbacks, and keep user hooks available during finalization. Flush after exit callbacks and propagate either stream failure to exit status 120. Assisted-by: OpenAI Codex:model version not exposed --- crates/vm/src/exceptions.rs | 20 +++-- crates/vm/src/stdlib/sys.rs | 134 +++++++++++++++----------------- crates/vm/src/vm/interpreter.rs | 18 +++-- crates/vm/src/vm/mod.rs | 97 +++++++---------------- crates/vm/src/vm/vm_object.rs | 41 +++++----- 5 files changed, 135 insertions(+), 175 deletions(-) diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index 774e6e55745..dd4c2f473ee 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -194,12 +194,7 @@ impl VirtualMachine { /// Display `exc` through `traceback._print_exception_bltin`, falling back /// to the native printer writing to `file`. pub(crate) fn display_exception_to(&self, file: &PyObject, exc: &Py) { - if let Ok(print_exception) = self - .import("traceback", 0) - .and_then(|traceback| traceback.get_attr("_print_exception_bltin", self)) - && print_exception.is_callable() - && print_exception.call((exc.to_owned(),), self).is_ok() - { + if self.try_display_exception_to(file, exc.as_object()) { return; } if self @@ -212,6 +207,19 @@ impl VirtualMachine { let _ = self.call_method(file, "flush", ()); } + /// Try the shared Python formatter with the caller's retained output file. + /// Reporting paths provide their own native fallback if it is unavailable. + pub(crate) fn try_display_exception_to(&self, file: &PyObject, exc: &PyObject) -> bool { + self.import("traceback", 0) + .and_then(|traceback| traceback.get_attr("_print_exception_bltin", self)) + .is_ok_and(|print_exception| { + print_exception.is_callable() + && print_exception + .call((exc.to_owned(), file.to_owned()), self) + .is_ok() + }) + } + pub fn write_exception( &self, output: &mut W, diff --git a/crates/vm/src/stdlib/sys.rs b/crates/vm/src/stdlib/sys.rs index 5ae0107779c..c72872a965a 100644 --- a/crates/vm/src/stdlib/sys.rs +++ b/crates/vm/src/stdlib/sys.rs @@ -45,7 +45,7 @@ pub mod sys { }, convert::ToPyObject, frame::FrameObjectRef, - function::{FuncArgs, KwArgs, OptionalArg, PosArgs}, + function::{FuncArgs, OptionalArg, PosArgs}, stdlib::_warnings::warn, types::PyStructSequence, version, @@ -1314,110 +1314,100 @@ pub mod sys { Ok(PyWindowsVersion::from_data(winver, vm)) } - fn _unraisablehook(unraisable: UnraisableHookArgsData, vm: &VirtualMachine) -> PyResult<()> { - use super::PyStderr; + /// Shared by sys.__unraisablehook__ and the fallback when a user hook fails. + /// Only the outer unraisable dispatcher discards reporting failures. + pub(crate) fn write_unraisable( + unraisable: UnraisableHookArgsData, + vm: &VirtualMachine, + ) -> PyResult<()> { + let Some(file) = vm.sys_module.dict().get_item_opt("stderr", vm)? else { + return Ok(()); + }; + if vm.is_none(&file) { + return Ok(()); + } + let write = |value: PyObjectRef| vm.call_method(&file, "write", (value,)).map(drop); + let text = |value: &str| write(vm.ctx.new_str(value).into()); - let stderr = PyStderr(vm); if !vm.is_none(&unraisable.object) { if !vm.is_none(&unraisable.err_msg) { - write!(stderr, "{}: ", unraisable.err_msg.str(vm)?); + write(unraisable.err_msg.str(vm)?.into())?; + text(": ")?; } else { - write!(stderr, "Exception ignored in: "); + text("Exception ignored in: ")?; } - // exception in del will be ignored but printed - let repr = &unraisable.object.repr(vm); - let str = match repr { - Ok(v) => v.to_string(), - Err(_) => format!( - "", - unraisable.object.class().name() - ), - }; - writeln!(stderr, "{str}"); + if unraisable + .object + .repr(vm) + .and_then(|value| write(value.into())) + .is_err() + { + text("")?; + } + text("\n")?; } else if !vm.is_none(&unraisable.err_msg) { - writeln!(stderr, "{}:", unraisable.err_msg.str(vm)?); + write(unraisable.err_msg.str(vm)?.into())?; + text(":\n")?; } - // Print traceback (using actual exc_traceback, not current stack) - if !vm.is_none(&unraisable.exc_traceback) { - let tb_module = vm.import("traceback", 0)?; - let print_tb = tb_module.get_attr("print_tb", vm)?; - let stderr_obj = super::get_stderr(vm)?; - let kwargs: KwArgs = core::iter::once(("file".to_string(), stderr_obj)).collect(); - let _ = print_tb.call( - FuncArgs::new(vec![unraisable.exc_traceback.clone()], kwargs), - vm, - ); + if vm.try_display_exception_to(&file, &unraisable.exc_value) { + return Ok(()); } - // Check exc_type - if vm.is_none(unraisable.exc_type.as_object()) { - return Ok(()); + // Imports can fail during teardown. Keep a native fallback, and keep + // printing the exception even if writing its traceback fails. + if let Ok(traceback) = unraisable + .exc_traceback + .downcast::() + { + let mut writer = crate::py_io::PyWriter(file.clone(), vm); + let _ = crate::exceptions::write_traceback(&mut writer, &traceback); } - assert!( - unraisable - .exc_type - .fast_issubclass(vm.ctx.exceptions.base_exception_type) - ); - - // Print module name (if not builtins or __main__). - // A failed lookup or a non-str `__module__` is discarded: print - // `` with no trailing `.`, then the qualname. - let module_name = unraisable.exc_type.__module__(vm).ok(); + + let module_name = unraisable + .exc_type + .as_object() + .get_attr("__module__", vm) + .ok(); if let Some(module_str) = module_name .as_ref() .and_then(|name| name.downcast_ref::()) { let module = module_str.as_wtf8(); if module != "builtins" && module != "__main__" { - write!(stderr, "{module}."); + write(module_str.to_owned().into())?; + text(".")?; } } else { - write!(stderr, ""); + text("")?; } - // Print qualname let qualname = unraisable.exc_type.__qualname__(vm); if let Ok(qualname_str) = qualname.downcast::() { - write!(stderr, "{}", qualname_str.as_wtf8()); + write(qualname_str.into())?; } else { - write!(stderr, "{}", unraisable.exc_type.name()); + text("")?; } - // Print exception value if !vm.is_none(&unraisable.exc_value) { - write!(stderr, ": "); - if let Ok(str) = unraisable.exc_value.str(vm) { - write!(stderr, "{}", str.as_wtf8()); - } else { - write!(stderr, ""); + text(": ")?; + if unraisable + .exc_value + .str(vm) + .and_then(|value| write(value.into())) + .is_err() + { + text("")?; } } - writeln!(stderr); - - // Flush stderr - if let Ok(stderr_obj) = super::get_stderr(vm) - && let Ok(flush) = stderr_obj.get_attr("flush", vm) - { - let _ = flush.call((), vm); - } - - Ok(()) + text("\n")?; + vm.call_method(&file, "flush", ()).map(drop) } #[pyattr] #[pyfunction(name = "__unraisablehook__")] - fn unraisablehook(unraisable: UnraisableHookArgsData, vm: &VirtualMachine) { - if let Err(e) = _unraisablehook(unraisable, vm) { - let stderr = super::PyStderr(vm); - writeln!( - stderr, - "{}", - e.as_object() - .repr(vm) - .unwrap_or_else(|_| vm.ctx.empty_str.to_owned()) - ); - } + fn unraisablehook(unraisable: UnraisableHookArgsData, vm: &VirtualMachine) -> PyResult<()> { + write_unraisable(unraisable, vm) } #[pyattr] diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index ba9e4f118ff..20567d6f347 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -724,11 +724,11 @@ impl Interpreter { /// Finalize vm and turns an exception to exit code. /// /// Finalization steps (matching Py_FinalizeEx): - /// 1. Flush stdout and stderr. /// 1. Handle exit exception and turn it to exit code. /// 1. Call threading._shutdown() to join non-daemon threads. /// 1. Run atexit exit functions. /// 1. Set finalizing flag and hang remaining daemon threads. + /// 1. Flush stdout and stderr. /// 1. Forced GC collection pass (collect cycles while builtins are available). /// 1. Module finalization (finalize_modules). /// 1. Clear interpreter-owned cross-interpreter data. @@ -748,13 +748,11 @@ impl Interpreter { fn finalize_inner(self, exit_code: impl FnOnce(&VirtualMachine) -> u32) -> u32 { self.enter(|vm| { - let flush_status = vm.flush_std(); - // See if any exception leaked out: let exit_code = exit_code(vm); - // Match CPython: if exit_code is 0 and stdout flush failed, exit 120 - if vm.finalize_ex(flush_status) < 0 && exit_code == 0 { + // A failed shutdown flush overrides even an explicit nonzero exit code. + if vm.finalize_ex() < 0 { EXITCODE_FLUSH_FAILURE } else { exit_code @@ -771,7 +769,7 @@ impl VirtualMachine { if self.state.thread_is_running_main() { self.state.set_not_running_main(); } - let status = if self.finalize_ex(self.flush_std()) < 0 { + let status = if self.finalize_ex() < 0 { EXITCODE_FLUSH_FAILURE } else { status @@ -779,9 +777,9 @@ impl VirtualMachine { crate::host_env::os::exit(status as i32) } - /// The steps of Py_FinalizeEx after the first flush of stdout and stderr. + /// The steps of Py_FinalizeEx. /// Return a negative value if any flush failed. - fn finalize_ex(&self, mut flush_status: i32) -> i32 { + fn finalize_ex(&self) -> i32 { let vm = self; // Wait for thread shutdown - call threading._shutdown() if available. // This waits for all non-daemon threads to complete. @@ -835,6 +833,10 @@ impl VirtualMachine { vm.state.finalizing.store(true, Ordering::Release); } + // Exit callbacks have finished, but modules and the user unraisable + // hook are still available to report a failed stream flush. + let mut flush_status = vm.flush_std(); + // GC pass - collect cycles before module cleanup vm.state.gc.collect_force(2); diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 7f075103f8c..3439fde7e65 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -1878,17 +1878,7 @@ impl VirtualMachine { object: PyObjectRef, ) { let write_original = || { - let msg = msg.as_ref().map(|msg| msg.to_string_lossy()); - self.write_unraisable_to_stderr(&e, msg.as_deref(), &object); - }; - // During finalization the hook may no longer exist. - if self.state.finalizing.load(Ordering::Acquire) { - write_original(); - return; - } - let Some(unraisablehook) = self.sys_module.get_attr("unraisablehook", self).ok() else { - write_original(); - return; + self.write_unraisable_to_stderr(&e, msg.clone(), &object); }; let args = stdlib::sys::UnraisableHookArgsData { exc_type: e.class().to_owned(), @@ -1898,12 +1888,27 @@ impl VirtualMachine { object: object.clone(), } .to_pyobject(self); + let unraisablehook = match self.sys_module.dict().get_item_opt("unraisablehook", self) { + Ok(Some(hook)) => hook, + Ok(None) => { + write_original(); + return; + } + Err(error) => { + self.write_unraisable_to_stderr( + &error, + Some(self.ctx.new_str("Exception ignored in sys.unraisablehook")), + &self.ctx.none(), + ); + return; + } + }; if let Err(error) = self.audit("sys.unraisablehook", || { (unraisablehook.clone(), args.clone()) }) { self.write_unraisable_to_stderr( &error, - Some("Exception ignored in audit hook"), + Some(self.ctx.new_str("Exception ignored in audit hook")), &self.ctx.none(), ); return; @@ -1915,7 +1920,7 @@ impl VirtualMachine { if let Err(error) = unraisablehook.call((args,), self) { self.write_unraisable_to_stderr( &error, - Some("Exception ignored in sys.unraisablehook"), + Some(self.ctx.new_str("Exception ignored in sys.unraisablehook")), &unraisablehook, ); } @@ -1926,67 +1931,17 @@ impl VirtualMachine { fn write_unraisable_to_stderr( &self, e: &Py, - msg: Option<&str>, + msg: Option, object: &PyObject, ) { - // Get stderr once and reuse it - let stderr = crate::stdlib::sys::get_stderr(self).ok(); - - let write_to_stderr = |s: &str, stderr: &Option, vm: &Self| { - if let Some(stderr) = stderr { - let _ = vm.call_method(stderr, "write", (s.to_owned(),)); - } else { - eprint!("{s}"); - } - }; - - if self.is_none(object) { - if let Some(msg) = msg { - write_to_stderr(&format!("{msg}:\n"), &stderr, self); - } - } else { - let msg_str = if let Some(msg) = msg { - format!("{msg}: ") - } else { - "Exception ignored in: ".to_owned() - }; - write_to_stderr(&msg_str, &stderr, self); - - let repr_result = object.repr(self); - let repr_wtf8 = repr_result - .as_ref() - .map_or_else(|_| "".as_ref(), |s| s.as_wtf8()); - write_to_stderr(&format!("{repr_wtf8}\n"), &stderr, self); - } - - // Print the existing traceback without importing Python modules; - // this path must also work while the interpreter is finalizing. - if let Some(traceback) = e.traceback() { - if let Some(stderr) = &stderr { - let mut writer = crate::py_io::PyWriter(stderr.clone(), self); - let _ = crate::exceptions::write_traceback(&mut writer, &traceback); - } else { - let mut writer = crate::py_io::IoWriter(std::io::stderr()); - let _ = crate::exceptions::write_traceback(&mut writer, &traceback); - } - } - let exc_type_name = e - .class() - .fully_qualified_name(self) - .unwrap_or_else(|_| e.class().name().to_string()); - let message = match e.as_object().str(self) { - Ok(value) if !value.as_wtf8().is_empty() => { - format!("{exc_type_name}: {}\n", value.as_wtf8()) - } - Ok(_) => format!("{exc_type_name}\n"), - Err(_) => format!("{exc_type_name}: \n"), + let args = stdlib::sys::UnraisableHookArgsData { + exc_type: e.class().to_owned(), + exc_value: e.to_owned().into(), + exc_traceback: e.traceback().to_pyobject(self), + err_msg: msg.to_pyobject(self), + object: object.to_owned(), }; - write_to_stderr(&message, &stderr, self); - - // Flush stderr to ensure output is visible - if let Some(ref stderr) = stderr { - let _ = self.call_method(stderr, "flush", ()); - } + let _ = stdlib::sys::sys::write_unraisable(args, self); } /// Store a callee frame pointer for the trampoline to pick up after diff --git a/crates/vm/src/vm/vm_object.rs b/crates/vm/src/vm/vm_object.rs index db9459b9b75..dcaf2147e49 100644 --- a/crates/vm/src/vm/vm_object.rs +++ b/crates/vm/src/vm/vm_object.rs @@ -3,7 +3,6 @@ use crate::{ builtins::{PyBaseExceptionRef, PyList, PyStrInterned, pystr::AsPyStr}, function::IntoFuncArgs, object::{AsObject, PyObject, PyObjectRef, PyResult}, - stdlib::sys, vm::VirtualMachine, }; @@ -68,23 +67,29 @@ impl VirtualMachine { } pub(crate) fn flush_std(&self) -> i32 { - let vm = self; - let status = if let Ok(stdout) = sys::get_stdout(vm) - && !vm.is_none(&stdout) - && !vm.file_is_closed(&stdout) - && let Err(e) = vm.call_method(&stdout, identifier!(vm, flush).as_str(), ()) - { - vm.run_unraisable(e, None, stdout); - -1 - } else { - 0 - }; - - if let Ok(stderr) = sys::get_stderr(vm) - && !vm.is_none(&stderr) - && !vm.file_is_closed(&stderr) - { - let _ = vm.call_method(&stderr, identifier!(vm, flush).as_str(), ()); + let mut status = 0; + for name in ["stdout", "stderr"] { + // Keep the stream alive until its failure has been reported: a + // flush or hook can replace the reference stored in sys.__dict__. + let file = self.sys_module.dict().get_item_opt(name, self); + let result = match &file { + Ok(Some(file)) if !self.is_none(file) && !self.file_is_closed(file) => { + self.call_method(file, "flush", ()).map(drop) + } + Ok(_) => Ok(()), + Err(error) => Err(error.clone()), + }; + if let Err(error) = result { + status = -1; + if name == "stdout" { + self.run_unraisable( + error, + Some("Exception ignored while flushing sys.stdout".to_owned()), + self.ctx.none(), + ); + } + // Reporting a stderr failure could require the broken stream. + } } status }