From f0c427b2d2d98dd2eb6dea62d27c8332fdb3a363 Mon Sep 17 00:00:00 2001 From: Moreal Date: Mon, 16 Jan 2023 00:23:41 +0900 Subject: [PATCH 1/4] Correct exitcode when running through `subprocess.run` --- vm/src/vm/mod.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/vm/src/vm/mod.rs b/vm/src/vm/mod.rs index c7eea2514..96be3048b 100644 --- a/vm/src/vm/mod.rs +++ b/vm/src/vm/mod.rs @@ -36,6 +36,11 @@ use crate::{ AsObject, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, }; use crossbeam_utils::atomic::AtomicCell; +#[cfg(unix)] +use nix::{ + sys::signal::{kill, sigaction, SaFlags, SigAction, SigSet, Signal::SIGINT}, + unistd::getpid, +}; use std::sync::atomic::AtomicBool; use std::{ borrow::Cow, @@ -763,6 +768,37 @@ impl VirtualMachine { self.print_exception(exc); #[cfg(unix)] { + unsafe { + if sigaction( + SIGINT, + &SigAction::new( + nix::sys::signal::SigHandler::SigDfl, + SaFlags::SA_ONSTACK, + SigSet::empty(), + ), + ) + .is_ok() + { + // FIXME: flush sys.stdout and sys.stderr before killing process. + if let Ok(stdout) = stdlib::sys::get_stdout(self) { + let _ = self.call_method( + &stdout, + identifier!(self, flush).as_str(), + (), + ); + } + if let Ok(stderr) = stdlib::sys::get_stderr(self) { + let _ = self.call_method( + &stderr, + identifier!(self, flush).as_str(), + (), + ); + } + + kill(getpid(), SIGINT).expect("Expect to be killed."); + } + } + (libc::SIGINT as u8) + 128u8 } #[cfg(not(unix))] From 52afc1ace5e32de989860a7cbf16b978d8518ffb Mon Sep 17 00:00:00 2001 From: Moreal Date: Wed, 18 Jan 2023 14:33:37 +0900 Subject: [PATCH 2/4] Reuse flush_std --- vm/src/vm/interpreter.rs | 2 +- vm/src/vm/mod.rs | 17 +---------------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/vm/src/vm/interpreter.rs b/vm/src/vm/interpreter.rs index 8d0d85ea5..1a1a6cc97 100644 --- a/vm/src/vm/interpreter.rs +++ b/vm/src/vm/interpreter.rs @@ -84,7 +84,7 @@ impl Interpreter { } } -fn flush_std(vm: &VirtualMachine) { +pub(crate) fn flush_std(vm: &VirtualMachine) { if let Ok(stdout) = sys::get_stdout(vm) { let _ = vm.call_method(&stdout, identifier!(vm, flush).as_str(), ()); } diff --git a/vm/src/vm/mod.rs b/vm/src/vm/mod.rs index 96be3048b..b8d4d9d26 100644 --- a/vm/src/vm/mod.rs +++ b/vm/src/vm/mod.rs @@ -779,22 +779,7 @@ impl VirtualMachine { ) .is_ok() { - // FIXME: flush sys.stdout and sys.stderr before killing process. - if let Ok(stdout) = stdlib::sys::get_stdout(self) { - let _ = self.call_method( - &stdout, - identifier!(self, flush).as_str(), - (), - ); - } - if let Ok(stderr) = stdlib::sys::get_stderr(self) { - let _ = self.call_method( - &stderr, - identifier!(self, flush).as_str(), - (), - ); - } - + interpreter::flush_std(self); kill(getpid(), SIGINT).expect("Expect to be killed."); } } From 189a490e27a8a12e93b8b82822100fe197c5dbeb Mon Sep 17 00:00:00 2001 From: Moreal Date: Wed, 18 Jan 2023 15:58:40 +0900 Subject: [PATCH 3/4] Unmark resolved tests --- Lib/test/test_runpy.py | 25 +++++++++++++------------ Lib/test/test_signal.py | 2 -- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_runpy.py b/Lib/test/test_runpy.py index 0a4da9c82..48e25a06b 100644 --- a/Lib/test/test_runpy.py +++ b/Lib/test/test_runpy.py @@ -790,13 +790,9 @@ class TestExit(unittest.TestCase): self.assertTrue(proc.stderr.endswith("\nKeyboardInterrupt\n")) self.assertEqual(proc.returncode, self.EXPECTED_CODE) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_pymain_run_file(self): self.assertSigInt([sys.executable, self.ham]) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_pymain_run_file_runpy_run_module(self): tmp = self.ham.parent run_module = tmp / "run_module.py" @@ -810,8 +806,6 @@ class TestExit(unittest.TestCase): ) self.assertSigInt([sys.executable, run_module], cwd=tmp) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_pymain_run_file_runpy_run_module_as_main(self): tmp = self.ham.parent run_module_as_main = tmp / "run_module_as_main.py" @@ -825,16 +819,12 @@ class TestExit(unittest.TestCase): ) self.assertSigInt([sys.executable, run_module_as_main], cwd=tmp) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_pymain_run_command_run_module(self): self.assertSigInt( [sys.executable, "-c", "import runpy; runpy.run_module('ham')"], cwd=self.ham.parent, ) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_pymain_run_command(self): self.assertSigInt([sys.executable, "-c", "import ham"], cwd=self.ham.parent) @@ -843,12 +833,23 @@ class TestExit(unittest.TestCase): def test_pymain_run_stdin(self): self.assertSigInt([sys.executable], input="import ham", cwd=self.ham.parent) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_pymain_run_module(self): ham = self.ham self.assertSigInt([sys.executable, "-m", ham.stem], cwd=ham.parent) + # TODO: RUSTPYTHON + if sys.platform == "win32": + windows_only_failed_tests = [ + test_pymain_run_file, + test_pymain_run_file_runpy_run_module, + test_pymain_run_file_runpy_run_module_as_main, + test_pymain_run_command_run_module, + test_pymain_run_command, + test_pymain_run_module + ] + for t in windows_only_failed_tests: + unittest.expectedFailure(t) + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py index b39866be8..f8122f061 100644 --- a/Lib/test/test_signal.py +++ b/Lib/test/test_signal.py @@ -96,8 +96,6 @@ class PosixTests(unittest.TestCase): self.assertNotIn(signal.NSIG, s) self.assertLess(len(s), signal.NSIG) - # TODO: RUSTPYTHON - @unittest.expectedFailure @unittest.skipUnless(sys.executable, "sys.executable required.") def test_keyboard_interrupt_exit_code(self): """KeyboardInterrupt triggers exit via SIGINT.""" From a3507e7f58aaa13897cb5d15608ab4499f50f415 Mon Sep 17 00:00:00 2001 From: Moreal Date: Wed, 18 Jan 2023 22:07:42 +0900 Subject: [PATCH 4/4] Use `unsafe` only where necessary --- vm/src/vm/mod.rs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/vm/src/vm/mod.rs b/vm/src/vm/mod.rs index b8d4d9d26..99fe30489 100644 --- a/vm/src/vm/mod.rs +++ b/vm/src/vm/mod.rs @@ -768,20 +768,15 @@ impl VirtualMachine { self.print_exception(exc); #[cfg(unix)] { - unsafe { - if sigaction( - SIGINT, - &SigAction::new( - nix::sys::signal::SigHandler::SigDfl, - SaFlags::SA_ONSTACK, - SigSet::empty(), - ), - ) - .is_ok() - { - interpreter::flush_std(self); - kill(getpid(), SIGINT).expect("Expect to be killed."); - } + let action = SigAction::new( + nix::sys::signal::SigHandler::SigDfl, + SaFlags::SA_ONSTACK, + SigSet::empty(), + ); + let result = unsafe { sigaction(SIGINT, &action) }; + if result.is_ok() { + interpreter::flush_std(self); + kill(getpid(), SIGINT).expect("Expect to be killed."); } (libc::SIGINT as u8) + 128u8