From 1b17926e913140f946c3b102edad10532f33a933 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sun, 28 Jul 2019 21:55:08 +0300 Subject: [PATCH 01/27] Initial signal support --- vm/src/frame.rs | 1 + vm/src/stdlib/mod.rs | 3 +++ vm/src/stdlib/signal.rs | 37 +++++++++++++++++++++++++++++++++++++ vm/src/vm.rs | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 vm/src/stdlib/signal.rs diff --git a/vm/src/frame.rs b/vm/src/frame.rs index b70cb658c4..969327b112 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -163,6 +163,7 @@ impl Frame { /// Execute a single instruction. #[allow(clippy::cognitive_complexity)] fn execute_instruction(&self, vm: &VirtualMachine) -> FrameResult { + vm.check_signals(); let instruction = self.fetch_instruction(); flame_guard!(format!("Frame::execute_instruction({:?})", instruction)); diff --git a/vm/src/stdlib/mod.rs b/vm/src/stdlib/mod.rs index dea27dfe08..ae0f0580a1 100644 --- a/vm/src/stdlib/mod.rs +++ b/vm/src/stdlib/mod.rs @@ -38,6 +38,8 @@ pub mod io; mod os; #[cfg(all(unix, not(any(target_os = "android", target_os = "redox"))))] mod pwd; +#[cfg(not(target_arch = "wasm32"))] +mod signal; use crate::pyobject::PyObjectRef; @@ -92,6 +94,7 @@ pub fn get_module_inits() -> HashMap { modules.insert("_io".to_string(), Box::new(io::make_module)); modules.insert("_os".to_string(), Box::new(os::make_module)); modules.insert("socket".to_string(), Box::new(socket::make_module)); + modules.insert("_signal".to_string(), Box::new(signal::make_module)); } // Unix-only diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs new file mode 100644 index 0000000000..bd054fe972 --- /dev/null +++ b/vm/src/stdlib/signal.rs @@ -0,0 +1,37 @@ +use crate::obj::objfunction::PyFunctionRef; +use crate::obj::objint::PyIntRef; +use crate::pyobject::PyObjectRef; +use crate::pyobject::PyResult; +use crate::vm::{VirtualMachine, TRIGGERS}; + +use std::sync::atomic::Ordering; + +use num_traits::cast::ToPrimitive; + +use nix::sys::signal; + +extern "C" fn run_signal(signum: i32) { + unsafe { + TRIGGERS[signum as usize].store(true, Ordering::Relaxed); + } +} + +fn signal(signalnum: PyIntRef, handler: PyFunctionRef, vm: &VirtualMachine) -> PyResult<()> { + vm.signal_handlers.borrow_mut().insert( + signalnum.as_bigint().to_i32().unwrap(), + handler.into_object(), + ); + let handler = nix::sys::signal::SigHandler::Handler(run_signal); + let sig_action = + signal::SigAction::new(handler, signal::SaFlags::empty(), signal::SigSet::empty()); + unsafe { signal::sigaction(signal::SIGINT, &sig_action) }.unwrap(); + Ok(()) +} + +pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { + let ctx = &vm.ctx; + + py_module!(vm, "_signal", { + "signal" => ctx.new_rustfunc(signal), + }) +} diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 4c231eebde..07b2831933 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -9,6 +9,7 @@ use std::collections::hash_map::HashMap; use std::collections::hash_set::HashSet; use std::fmt; use std::rc::Rc; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Mutex, MutexGuard}; use crate::builtins; @@ -42,6 +43,23 @@ use num_bigint::BigInt; #[cfg(feature = "rustpython-compiler")] use rustpython_compiler::{compile, error::CompileError}; +// Signal triggers +// TODO: 64 +pub const NSIG: usize = 10; + +pub static mut TRIGGERS: [AtomicBool; NSIG] = [ + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), +]; + // use objects::objects; // Objects are live when they are on stack, or referenced by a name (for now) @@ -62,6 +80,7 @@ pub struct VirtualMachine { pub trace_func: RefCell, pub use_tracing: RefCell, pub settings: PySettings, + pub signal_handlers: RefCell>, } /// Struct containing all kind of settings for the python vm. @@ -160,6 +179,7 @@ impl VirtualMachine { trace_func, use_tracing: RefCell::new(false), settings, + signal_handlers: RefCell::new(HashMap::new()), }; builtins::make_module(&vm, builtins.clone()); @@ -1143,6 +1163,21 @@ impl VirtualMachine { pub fn current_exception(&self) -> Option { self.exceptions.borrow().last().cloned() } + + pub fn check_signals(&self) { + for (signum, handler) in self.signal_handlers.borrow().iter() { + if *signum as usize >= NSIG { + panic!("Signum bigger then NSIG"); + } + let triggerd; + unsafe { + triggerd = TRIGGERS[*signum as usize].swap(false, Ordering::Relaxed); + } + if triggerd { + self.invoke(handler.clone(), vec![]).expect("Test"); + } + } + } } impl Default for VirtualMachine { From 5b670f8a26e588b0e4f28cd258e50d86378aba77 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Mon, 29 Jul 2019 18:39:26 +0300 Subject: [PATCH 02/27] Simplify signal code --- vm/src/vm.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 07b2831933..15978d4c0f 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -179,7 +179,7 @@ impl VirtualMachine { trace_func, use_tracing: RefCell::new(false), settings, - signal_handlers: RefCell::new(HashMap::new()), + signal_handlers: Default::default(), }; builtins::make_module(&vm, builtins.clone()); @@ -1169,10 +1169,7 @@ impl VirtualMachine { if *signum as usize >= NSIG { panic!("Signum bigger then NSIG"); } - let triggerd; - unsafe { - triggerd = TRIGGERS[*signum as usize].swap(false, Ordering::Relaxed); - } + let triggerd = unsafe { TRIGGERS[*signum as usize].swap(false, Ordering::Relaxed) }; if triggerd { self.invoke(handler.clone(), vec![]).expect("Test"); } From f540d31a1b2a5acece3d765b93b887049160726a Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Mon, 29 Jul 2019 18:46:51 +0300 Subject: [PATCH 03/27] Move static triggers array to signal.rs --- vm/src/frame.rs | 4 +++- vm/src/stdlib/mod.rs | 2 +- vm/src/stdlib/signal.rs | 33 +++++++++++++++++++++++++++++++-- vm/src/vm.rs | 30 ------------------------------ 4 files changed, 35 insertions(+), 34 deletions(-) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 969327b112..0b67df1dcb 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -19,6 +19,8 @@ use crate::pyobject::{ IdProtocol, ItemProtocol, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; use crate::scope::{NameProtocol, Scope}; +#[cfg(not(target_arch = "wasm32"))] +use crate::stdlib::signal::check_signals; use crate::vm::VirtualMachine; use indexmap::IndexMap; use itertools::Itertools; @@ -163,7 +165,7 @@ impl Frame { /// Execute a single instruction. #[allow(clippy::cognitive_complexity)] fn execute_instruction(&self, vm: &VirtualMachine) -> FrameResult { - vm.check_signals(); + check_signals(vm); let instruction = self.fetch_instruction(); flame_guard!(format!("Frame::execute_instruction({:?})", instruction)); diff --git a/vm/src/stdlib/mod.rs b/vm/src/stdlib/mod.rs index ae0f0580a1..4e39081a02 100644 --- a/vm/src/stdlib/mod.rs +++ b/vm/src/stdlib/mod.rs @@ -39,7 +39,7 @@ mod os; #[cfg(all(unix, not(any(target_os = "android", target_os = "redox"))))] mod pwd; #[cfg(not(target_arch = "wasm32"))] -mod signal; +pub mod signal; use crate::pyobject::PyObjectRef; diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index bd054fe972..8a86043276 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -2,14 +2,31 @@ use crate::obj::objfunction::PyFunctionRef; use crate::obj::objint::PyIntRef; use crate::pyobject::PyObjectRef; use crate::pyobject::PyResult; -use crate::vm::{VirtualMachine, TRIGGERS}; +use crate::vm::VirtualMachine; -use std::sync::atomic::Ordering; +use std::sync::atomic::{AtomicBool, Ordering}; use num_traits::cast::ToPrimitive; use nix::sys::signal; +// Signal triggers +// TODO: 64 +const NSIG: usize = 10; + +static mut TRIGGERS: [AtomicBool; NSIG] = [ + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), +]; + extern "C" fn run_signal(signum: i32) { unsafe { TRIGGERS[signum as usize].store(true, Ordering::Relaxed); @@ -28,6 +45,18 @@ fn signal(signalnum: PyIntRef, handler: PyFunctionRef, vm: &VirtualMachine) -> P Ok(()) } +pub fn check_signals(vm: &VirtualMachine) { + for (signum, handler) in vm.signal_handlers.borrow().iter() { + if *signum as usize >= NSIG { + panic!("Signum bigger then NSIG"); + } + let triggerd = unsafe { TRIGGERS[*signum as usize].swap(false, Ordering::Relaxed) }; + if triggerd { + vm.invoke(handler.clone(), vec![]).expect("Test"); + } + } +} + pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { let ctx = &vm.ctx; diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 15978d4c0f..bbfea7fb60 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -9,7 +9,6 @@ use std::collections::hash_map::HashMap; use std::collections::hash_set::HashSet; use std::fmt; use std::rc::Rc; -use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Mutex, MutexGuard}; use crate::builtins; @@ -43,23 +42,6 @@ use num_bigint::BigInt; #[cfg(feature = "rustpython-compiler")] use rustpython_compiler::{compile, error::CompileError}; -// Signal triggers -// TODO: 64 -pub const NSIG: usize = 10; - -pub static mut TRIGGERS: [AtomicBool; NSIG] = [ - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), -]; - // use objects::objects; // Objects are live when they are on stack, or referenced by a name (for now) @@ -1163,18 +1145,6 @@ impl VirtualMachine { pub fn current_exception(&self) -> Option { self.exceptions.borrow().last().cloned() } - - pub fn check_signals(&self) { - for (signum, handler) in self.signal_handlers.borrow().iter() { - if *signum as usize >= NSIG { - panic!("Signum bigger then NSIG"); - } - let triggerd = unsafe { TRIGGERS[*signum as usize].swap(false, Ordering::Relaxed) }; - if triggerd { - self.invoke(handler.clone(), vec![]).expect("Test"); - } - } - } } impl Default for VirtualMachine { From ca23c43ae4edaeeb4471c8e09fc0526fa6e9fc41 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Mon, 29 Jul 2019 19:13:01 +0300 Subject: [PATCH 04/27] Return previous handler from signal --- vm/src/stdlib/signal.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index 8a86043276..7298b5cadb 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -33,16 +33,25 @@ extern "C" fn run_signal(signum: i32) { } } -fn signal(signalnum: PyIntRef, handler: PyFunctionRef, vm: &VirtualMachine) -> PyResult<()> { - vm.signal_handlers.borrow_mut().insert( - signalnum.as_bigint().to_i32().unwrap(), - handler.into_object(), +fn signal( + signalnum: PyIntRef, + handler: PyFunctionRef, + vm: &VirtualMachine, +) -> PyResult> { + let signalnum = signalnum.as_bigint().to_i32().unwrap(); + let signal_enum = signal::Signal::from_c_int(signalnum).unwrap(); + let sig_handler = nix::sys::signal::SigHandler::Handler(run_signal); + let sig_action = signal::SigAction::new( + sig_handler, + signal::SaFlags::empty(), + signal::SigSet::empty(), ); - let handler = nix::sys::signal::SigHandler::Handler(run_signal); - let sig_action = - signal::SigAction::new(handler, signal::SaFlags::empty(), signal::SigSet::empty()); - unsafe { signal::sigaction(signal::SIGINT, &sig_action) }.unwrap(); - Ok(()) + unsafe { signal::sigaction(signal_enum, &sig_action) }.unwrap(); + let old_handler = vm + .signal_handlers + .borrow_mut() + .insert(signalnum, handler.into_object()); + Ok(old_handler) } pub fn check_signals(vm: &VirtualMachine) { From 57cdae17a7daed54750257e6e248360685dc43ec Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Mon, 29 Jul 2019 20:46:40 +0300 Subject: [PATCH 05/27] Add signal.getsignal --- vm/src/stdlib/signal.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index 7298b5cadb..6b39b8f51a 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -46,6 +46,7 @@ fn signal( signal::SaFlags::empty(), signal::SigSet::empty(), ); + check_signals(vm); unsafe { signal::sigaction(signal_enum, &sig_action) }.unwrap(); let old_handler = vm .signal_handlers @@ -54,6 +55,15 @@ fn signal( Ok(old_handler) } +fn getsignal(signalnum: PyIntRef, vm: &VirtualMachine) -> PyResult> { + let signalnum = signalnum.as_bigint().to_i32().unwrap(); + Ok(vm + .signal_handlers + .borrow_mut() + .get(&signalnum) + .map(|x| x.clone())) +} + pub fn check_signals(vm: &VirtualMachine) { for (signum, handler) in vm.signal_handlers.borrow().iter() { if *signum as usize >= NSIG { @@ -71,5 +81,6 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { py_module!(vm, "_signal", { "signal" => ctx.new_rustfunc(signal), + "getsignal" => ctx.new_rustfunc(getsignal) }) } From a7d96b72235efc79d50e612e2e393b1048c58e47 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Mon, 29 Jul 2019 20:47:54 +0300 Subject: [PATCH 06/27] Use rust signal module directly --- vm/src/stdlib/mod.rs | 2 +- vm/src/stdlib/signal.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vm/src/stdlib/mod.rs b/vm/src/stdlib/mod.rs index 4e39081a02..b41342c229 100644 --- a/vm/src/stdlib/mod.rs +++ b/vm/src/stdlib/mod.rs @@ -94,7 +94,7 @@ pub fn get_module_inits() -> HashMap { modules.insert("_io".to_string(), Box::new(io::make_module)); modules.insert("_os".to_string(), Box::new(os::make_module)); modules.insert("socket".to_string(), Box::new(socket::make_module)); - modules.insert("_signal".to_string(), Box::new(signal::make_module)); + modules.insert("signal".to_string(), Box::new(signal::make_module)); } // Unix-only diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index 6b39b8f51a..c7e66a7dc9 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -79,7 +79,7 @@ pub fn check_signals(vm: &VirtualMachine) { pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { let ctx = &vm.ctx; - py_module!(vm, "_signal", { + py_module!(vm, "signal", { "signal" => ctx.new_rustfunc(signal), "getsignal" => ctx.new_rustfunc(getsignal) }) From e8001d789ffb27a6f4c7cca038ccd240afb680be Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Mon, 29 Jul 2019 21:01:47 +0300 Subject: [PATCH 07/27] Add signal.alarm --- vm/src/stdlib/signal.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index c7e66a7dc9..c26756d908 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -9,6 +9,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use num_traits::cast::ToPrimitive; use nix::sys::signal; +use nix::unistd::alarm as sig_alarm; // Signal triggers // TODO: 64 @@ -64,6 +65,16 @@ fn getsignal(signalnum: PyIntRef, vm: &VirtualMachine) -> PyResult u32 { + let time = time.as_bigint().to_u32().unwrap(); + let prev_time = if time == 0 { + sig_alarm::cancel() + } else { + sig_alarm::set(time) + }; + prev_time.unwrap_or(0) +} + pub fn check_signals(vm: &VirtualMachine) { for (signum, handler) in vm.signal_handlers.borrow().iter() { if *signum as usize >= NSIG { @@ -81,6 +92,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { py_module!(vm, "signal", { "signal" => ctx.new_rustfunc(signal), - "getsignal" => ctx.new_rustfunc(getsignal) + "getsignal" => ctx.new_rustfunc(getsignal), + "alarm" => ctx.new_rustfunc(alarm), }) } From c1e07999cf3c4cd05c84493ae425aa22eaf7543c Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Mon, 29 Jul 2019 21:13:55 +0300 Subject: [PATCH 08/27] Add test for signal --- tests/snippets/stdlib_signal.py | 16 ++++++++++++++++ vm/src/stdlib/signal.rs | 10 ++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 tests/snippets/stdlib_signal.py diff --git a/tests/snippets/stdlib_signal.py b/tests/snippets/stdlib_signal.py new file mode 100644 index 0000000000..d711052eb5 --- /dev/null +++ b/tests/snippets/stdlib_signal.py @@ -0,0 +1,16 @@ +import signal +import time + +signals = [] + +def handler(signum, frame): + signals.append(signum) + + +signal.signal(14, handler) +assert signal.getsignal(14) is handler + +signal.alarm(2) +time.sleep(3.0) +assert signals == [14] + diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index c26756d908..01e047fc87 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -13,7 +13,7 @@ use nix::unistd::alarm as sig_alarm; // Signal triggers // TODO: 64 -const NSIG: usize = 10; +const NSIG: usize = 15; static mut TRIGGERS: [AtomicBool; NSIG] = [ AtomicBool::new(false), @@ -26,6 +26,11 @@ static mut TRIGGERS: [AtomicBool; NSIG] = [ AtomicBool::new(false), AtomicBool::new(false), AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), + AtomicBool::new(false), ]; extern "C" fn run_signal(signum: i32) { @@ -82,7 +87,8 @@ pub fn check_signals(vm: &VirtualMachine) { } let triggerd = unsafe { TRIGGERS[*signum as usize].swap(false, Ordering::Relaxed) }; if triggerd { - vm.invoke(handler.clone(), vec![]).expect("Test"); + vm.invoke(handler.clone(), vec![vm.new_int(*signum), vm.get_none()]) + .expect("Test"); } } } From 7061f1c9c4bcd4336d359b97f7f85d01134c91b6 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Wed, 31 Jul 2019 19:22:11 +0300 Subject: [PATCH 09/27] Add signal.{SIG_IGN, SIG_DFL} --- vm/src/stdlib/signal.rs | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index 01e047fc87..557bf67b86 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -1,7 +1,5 @@ -use crate::obj::objfunction::PyFunctionRef; use crate::obj::objint::PyIntRef; -use crate::pyobject::PyObjectRef; -use crate::pyobject::PyResult; +use crate::pyobject::{IdProtocol, PyObjectRef, PyResult}; use crate::vm::VirtualMachine; use std::sync::atomic::{AtomicBool, Ordering}; @@ -41,12 +39,27 @@ extern "C" fn run_signal(signum: i32) { fn signal( signalnum: PyIntRef, - handler: PyFunctionRef, + handler: PyObjectRef, vm: &VirtualMachine, ) -> PyResult> { + if !vm.isinstance(&handler, &vm.ctx.function_type())? + && !vm.isinstance(&handler, &vm.ctx.bound_method_type())? + && !vm.isinstance(&handler, &vm.ctx.builtin_function_or_method_type())? + { + return Err(vm.new_type_error("Hanlder must be callable".to_string())); + } + let signal = vm.import("signal", &vm.ctx.new_tuple(vec![]), 0)?; + let sig_dfl = vm.get_attribute(signal.clone(), "SIG_DFL")?; + let sig_ign = vm.get_attribute(signal, "SIG_DFL")?; let signalnum = signalnum.as_bigint().to_i32().unwrap(); let signal_enum = signal::Signal::from_c_int(signalnum).unwrap(); - let sig_handler = nix::sys::signal::SigHandler::Handler(run_signal); + let sig_handler = if handler.is(&sig_dfl) { + signal::SigHandler::SigDfl + } else if handler.is(&sig_ign) { + signal::SigHandler::SigIgn + } else { + signal::SigHandler::Handler(run_signal) + }; let sig_action = signal::SigAction::new( sig_handler, signal::SaFlags::empty(), @@ -54,10 +67,7 @@ fn signal( ); check_signals(vm); unsafe { signal::sigaction(signal_enum, &sig_action) }.unwrap(); - let old_handler = vm - .signal_handlers - .borrow_mut() - .insert(signalnum, handler.into_object()); + let old_handler = vm.signal_handlers.borrow_mut().insert(signalnum, handler); Ok(old_handler) } @@ -93,12 +103,21 @@ pub fn check_signals(vm: &VirtualMachine) { } } +fn stub_func(_vm: &VirtualMachine) -> PyResult { + panic!("Do not use directly"); +} + pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { let ctx = &vm.ctx; + let sig_dfl = ctx.new_rustfunc(stub_func); + let sig_ign = ctx.new_rustfunc(stub_func); + py_module!(vm, "signal", { "signal" => ctx.new_rustfunc(signal), "getsignal" => ctx.new_rustfunc(getsignal), "alarm" => ctx.new_rustfunc(alarm), + "SIG_DFL" => sig_dfl, + "SIG_IGN" => sig_ign, }) } From a1af6b4d6c326193c18e193e65311ac1c441c109 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Wed, 31 Jul 2019 19:31:00 +0300 Subject: [PATCH 10/27] Iterate over triggers in check_sginals --- vm/src/stdlib/signal.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index 557bf67b86..8dd767afac 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -91,13 +91,16 @@ fn alarm(time: PyIntRef, _vm: &VirtualMachine) -> u32 { } pub fn check_signals(vm: &VirtualMachine) { - for (signum, handler) in vm.signal_handlers.borrow().iter() { - if *signum as usize >= NSIG { - panic!("Signum bigger then NSIG"); - } - let triggerd = unsafe { TRIGGERS[*signum as usize].swap(false, Ordering::Relaxed) }; + for signum in 1..NSIG { + let triggerd = unsafe { TRIGGERS[signum].swap(false, Ordering::Relaxed) }; if triggerd { - vm.invoke(handler.clone(), vec![vm.new_int(*signum), vm.get_none()]) + let handler = vm + .signal_handlers + .borrow() + .get(&(signum as i32)) + .expect("Handler should be set") + .clone(); + vm.invoke(handler, vec![vm.new_int(signum), vm.get_none()]) .expect("Test"); } } From 56b555b905f35674e5be40dbac5bd300ca0ee9c7 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Wed, 31 Jul 2019 19:48:47 +0300 Subject: [PATCH 11/27] Add signal numbers --- vm/src/stdlib/signal.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index 8dd767afac..d519c66dd9 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -122,5 +122,36 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { "alarm" => ctx.new_rustfunc(alarm), "SIG_DFL" => sig_dfl, "SIG_IGN" => sig_ign, + "SIGHUP" => ctx.new_int(signal::Signal::SIGHUP as u8), + "SIGINT" => ctx.new_int(signal::Signal::SIGINT as u8), + "SIGQUIT" => ctx.new_int(signal::Signal::SIGQUIT as u8), + "SIGILL" => ctx.new_int(signal::Signal::SIGILL as u8), + "SIGTRAP" => ctx.new_int(signal::Signal::SIGTRAP as u8), + "SIGABRT" => ctx.new_int(signal::Signal::SIGABRT as u8), + "SIGBUS" => ctx.new_int(signal::Signal::SIGBUS as u8), + "SIGFPE" => ctx.new_int(signal::Signal::SIGFPE as u8), + "SIGKILL" => ctx.new_int(signal::Signal::SIGKILL as u8), + "SIGUSR1" => ctx.new_int(signal::Signal::SIGUSR1 as u8), + "SIGSEGV" => ctx.new_int(signal::Signal::SIGSEGV as u8), + "SIGUSR2" => ctx.new_int(signal::Signal::SIGUSR2 as u8), + "SIGPIPE" => ctx.new_int(signal::Signal::SIGPIPE as u8), + "SIGALRM" => ctx.new_int(signal::Signal::SIGALRM as u8), + "SIGTERM" => ctx.new_int(signal::Signal::SIGTERM as u8), + "SIGSTKFLT" => ctx.new_int(signal::Signal::SIGSTKFLT as u8), + "SIGCHLD" => ctx.new_int(signal::Signal::SIGCHLD as u8), + "SIGCONT" => ctx.new_int(signal::Signal::SIGCONT as u8), + "SIGSTOP" => ctx.new_int(signal::Signal::SIGSTOP as u8), + "SIGTSTP" => ctx.new_int(signal::Signal::SIGTSTP as u8), + "SIGTTIN" => ctx.new_int(signal::Signal::SIGTTIN as u8), + "SIGTTOU" => ctx.new_int(signal::Signal::SIGTTOU as u8), + "SIGURG" => ctx.new_int(signal::Signal::SIGURG as u8), + "SIGXCPU" => ctx.new_int(signal::Signal::SIGXCPU as u8), + "SIGXFSZ" => ctx.new_int(signal::Signal::SIGXFSZ as u8), + "SIGVTALRM" => ctx.new_int(signal::Signal::SIGVTALRM as u8), + "SIGPROF" => ctx.new_int(signal::Signal::SIGPROF as u8), + "SIGWINCH" => ctx.new_int(signal::Signal::SIGWINCH as u8), + "SIGIO" => ctx.new_int(signal::Signal::SIGIO as u8), + "SIGPWR" => ctx.new_int(signal::Signal::SIGPWR as u8), + "SIGSYS" => ctx.new_int(signal::Signal::SIGSYS as u8), }) } From 785b5d8af7383aafe92ff57bb5771e0afff7d6d8 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Wed, 31 Jul 2019 20:03:08 +0300 Subject: [PATCH 12/27] Improve signal test --- tests/snippets/stdlib_signal.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/snippets/stdlib_signal.py b/tests/snippets/stdlib_signal.py index d711052eb5..703bc14ae2 100644 --- a/tests/snippets/stdlib_signal.py +++ b/tests/snippets/stdlib_signal.py @@ -1,5 +1,8 @@ import signal import time +from testutils import assert_raises + +assert_raises(TypeError, lambda: signal.signal(signal.SIGINT, 2)) signals = [] @@ -7,10 +10,24 @@ def handler(signum, frame): signals.append(signum) -signal.signal(14, handler) -assert signal.getsignal(14) is handler +signal.signal(signal.SIGALRM, handler) +assert signal.getsignal(signal.SIGALRM) is handler + +signal.alarm(1) +time.sleep(2.0) +assert signals == [signal.SIGALRM] + +signal.signal(signal.SIGALRM, signal.SIG_IGN) +signal.alarm(1) +time.sleep(2.0) + +assert signals == [signal.SIGALRM] + +signal.signal(signal.SIGALRM, handler) +signal.alarm(1) +time.sleep(2.0) + +assert signals == [signal.SIGALRM, signal.SIGALRM] + -signal.alarm(2) -time.sleep(3.0) -assert signals == [14] From 61bf0763dd1ee0719658cf46cfad2d0100c142b5 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Wed, 31 Jul 2019 21:42:53 +0300 Subject: [PATCH 13/27] User arr_macro to create triggers array --- Cargo.lock | 22 ++++++++++++++++++++++ vm/Cargo.toml | 1 + vm/src/stdlib/signal.rs | 24 ++++-------------------- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e05958928c..af20cbf093 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -33,6 +33,25 @@ dependencies = [ "scoped_threadpool 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "arr_macro" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "arr_macro_impl 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "proc-macro-hack 0.5.8 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "arr_macro_impl" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "proc-macro-hack 0.5.8 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 0.6.13 (registry+https://github.com/rust-lang/crates.io-index)", + "syn 0.15.39 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "arrayvec" version = "0.4.11" @@ -1011,6 +1030,7 @@ dependencies = [ name = "rustpython-vm" version = "0.1.0" dependencies = [ + "arr_macro 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "bincode 1.1.4 (registry+https://github.com/rust-lang/crates.io-index)", "bitflags 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "blake2 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1855,6 +1875,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum aho-corasick 0.7.4 (registry+https://github.com/rust-lang/crates.io-index)" = "36b7aa1ccb7d7ea3f437cf025a2ab1c47cc6c1bc9fc84918ff449def12f5e282" "checksum ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b" "checksum argon2rs 0.2.5 (registry+https://github.com/rust-lang/crates.io-index)" = "3f67b0b6a86dae6e67ff4ca2b6201396074996379fba2b92ff649126f37cb392" +"checksum arr_macro 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "d262b83f2f573121554ad6e764cd444303df85d86e5fcebc81903ddcf8dd3a97" +"checksum arr_macro_impl 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "8decbe97ffec939e44228d91e5d0829ceb1616c6ed0984c09df164b1e7ebaafc" "checksum arrayvec 0.4.11 (registry+https://github.com/rust-lang/crates.io-index)" = "b8d73f9beda665eaa98ab9e4f7442bd4e7de6652587de55b2525e52e29c1b0ba" "checksum ascii-canvas 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b385d69402821a1c254533a011a312531cbcc0e3e24f19bbb4747a5a2daf37e2" "checksum atty 0.2.13 (registry+https://github.com/rust-lang/crates.io-index)" = "1803c647a3ec87095e7ae7acfca019e98de5ec9a7d01343f611cf3152ed71a90" diff --git a/vm/Cargo.toml b/vm/Cargo.toml index d4913b782f..6ffc8c3782 100644 --- a/vm/Cargo.toml +++ b/vm/Cargo.toml @@ -63,6 +63,7 @@ bitflags = "1.1" libc = "0.2" nix = "0.14.1" wtf8 = "0.0.3" +arr_macro = "0.1.2" flame = { version = "0.2", optional = true } flamer = { version = "0.3", optional = true } diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index d519c66dd9..bf31c16e2b 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -6,30 +6,14 @@ use std::sync::atomic::{AtomicBool, Ordering}; use num_traits::cast::ToPrimitive; +use arr_macro::arr; use nix::sys::signal; use nix::unistd::alarm as sig_alarm; -// Signal triggers -// TODO: 64 -const NSIG: usize = 15; +const NSIG: usize = 64; -static mut TRIGGERS: [AtomicBool; NSIG] = [ - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), - AtomicBool::new(false), -]; +// We cannot use the NSIG const in the arr macro. This will fail compilation if NSIG is different. +static mut TRIGGERS: [AtomicBool; NSIG] = arr![AtomicBool::new(false); 64]; extern "C" fn run_signal(signum: i32) { unsafe { From 9470d75b5b63ab41cb902ada3a0a576f31a72d5c Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Wed, 31 Jul 2019 22:15:09 +0300 Subject: [PATCH 14/27] Compile nix parts only on unix --- vm/src/stdlib/signal.rs | 72 ++++++++++++++++++++++++++++++++--------- 1 file changed, 57 insertions(+), 15 deletions(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index bf31c16e2b..5ff4f17605 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -7,7 +7,10 @@ use std::sync::atomic::{AtomicBool, Ordering}; use num_traits::cast::ToPrimitive; use arr_macro::arr; + +#[cfg(unix)] use nix::sys::signal; +#[cfg(unix)] use nix::unistd::alarm as sig_alarm; const NSIG: usize = 64; @@ -21,6 +24,34 @@ extern "C" fn run_signal(signum: i32) { } } +#[derive(Debug)] +enum SigMode { + SigIgn, + SigDfl, + SigHandler, +} + +#[cfg(unix)] +fn os_set_signal(signalnum: i32, mode: SigMode, _vm: &VirtualMachine) { + let signal_enum = signal::Signal::from_c_int(signalnum).unwrap(); + let sig_handler = match mode { + SigMode::SigDfl => signal::SigHandler::SigDfl, + SigMode::SigIgn => signal::SigHandler::SigIgn, + SigMode::SigHandler => signal::SigHandler::Handler(run_signal), + }; + let sig_action = signal::SigAction::new( + sig_handler, + signal::SaFlags::empty(), + signal::SigSet::empty(), + ); + unsafe { signal::sigaction(signal_enum, &sig_action) }.unwrap(); +} + +#[cfg(not(unix))] +fn os_set_signal(_signalnum: i32, _mode: SigMode, _vm: &VirtualMachine) { + panic!("Not implemented"); +} + fn signal( signalnum: PyIntRef, handler: PyObjectRef, @@ -36,21 +67,15 @@ fn signal( let sig_dfl = vm.get_attribute(signal.clone(), "SIG_DFL")?; let sig_ign = vm.get_attribute(signal, "SIG_DFL")?; let signalnum = signalnum.as_bigint().to_i32().unwrap(); - let signal_enum = signal::Signal::from_c_int(signalnum).unwrap(); - let sig_handler = if handler.is(&sig_dfl) { - signal::SigHandler::SigDfl + check_signals(vm); + let mode = if handler.is(&sig_dfl) { + SigMode::SigDfl } else if handler.is(&sig_ign) { - signal::SigHandler::SigIgn + SigMode::SigIgn } else { - signal::SigHandler::Handler(run_signal) + SigMode::SigHandler }; - let sig_action = signal::SigAction::new( - sig_handler, - signal::SaFlags::empty(), - signal::SigSet::empty(), - ); - check_signals(vm); - unsafe { signal::sigaction(signal_enum, &sig_action) }.unwrap(); + os_set_signal(signalnum, mode, vm); let old_handler = vm.signal_handlers.borrow_mut().insert(signalnum, handler); Ok(old_handler) } @@ -64,6 +89,7 @@ fn getsignal(signalnum: PyIntRef, vm: &VirtualMachine) -> PyResult u32 { let time = time.as_bigint().to_u32().unwrap(); let prev_time = if time == 0 { @@ -100,12 +126,21 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { let sig_dfl = ctx.new_rustfunc(stub_func); let sig_ign = ctx.new_rustfunc(stub_func); - py_module!(vm, "signal", { + let module = py_module!(vm, "signal", { "signal" => ctx.new_rustfunc(signal), "getsignal" => ctx.new_rustfunc(getsignal), - "alarm" => ctx.new_rustfunc(alarm), "SIG_DFL" => sig_dfl, "SIG_IGN" => sig_ign, + }); + extend_module_platform_specific(vm, module) +} + +#[cfg(unix)] +fn extend_module_platform_specific(vm: &VirtualMachine, module: PyObjectRef) -> PyObjectRef { + let ctx = &vm.ctx; + + extend_module!(vm, module, { + "alarm" => ctx.new_rustfunc(alarm), "SIGHUP" => ctx.new_int(signal::Signal::SIGHUP as u8), "SIGINT" => ctx.new_int(signal::Signal::SIGINT as u8), "SIGQUIT" => ctx.new_int(signal::Signal::SIGQUIT as u8), @@ -137,5 +172,12 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { "SIGIO" => ctx.new_int(signal::Signal::SIGIO as u8), "SIGPWR" => ctx.new_int(signal::Signal::SIGPWR as u8), "SIGSYS" => ctx.new_int(signal::Signal::SIGSYS as u8), - }) + }); + + module +} + +#[cfg(not(unix))] +fn extend_module_platform_specific(_vm: &VirtualMachine, module: PyObjectRef) -> PyObjectRef { + module } From 48da527295d7005cad1acbda66edae79bbf2a05b Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Wed, 31 Jul 2019 22:18:49 +0300 Subject: [PATCH 15/27] Test signal only on unix --- tests/snippets/stdlib_signal.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/snippets/stdlib_signal.py b/tests/snippets/stdlib_signal.py index 703bc14ae2..933208a233 100644 --- a/tests/snippets/stdlib_signal.py +++ b/tests/snippets/stdlib_signal.py @@ -1,5 +1,6 @@ import signal import time +import sys from testutils import assert_raises assert_raises(TypeError, lambda: signal.signal(signal.SIGINT, 2)) @@ -10,24 +11,26 @@ def handler(signum, frame): signals.append(signum) -signal.signal(signal.SIGALRM, handler) -assert signal.getsignal(signal.SIGALRM) is handler +# unix +if "win" not in sys.platform: + signal.signal(signal.SIGALRM, handler) + assert signal.getsignal(signal.SIGALRM) is handler -signal.alarm(1) -time.sleep(2.0) -assert signals == [signal.SIGALRM] + signal.alarm(1) + time.sleep(2.0) + assert signals == [signal.SIGALRM] -signal.signal(signal.SIGALRM, signal.SIG_IGN) -signal.alarm(1) -time.sleep(2.0) + signal.signal(signal.SIGALRM, signal.SIG_IGN) + signal.alarm(1) + time.sleep(2.0) -assert signals == [signal.SIGALRM] + assert signals == [signal.SIGALRM] -signal.signal(signal.SIGALRM, handler) -signal.alarm(1) -time.sleep(2.0) + signal.signal(signal.SIGALRM, handler) + signal.alarm(1) + time.sleep(2.0) -assert signals == [signal.SIGALRM, signal.SIGALRM] + assert signals == [signal.SIGALRM, signal.SIGALRM] From f3b4b28ec2891c50bc3275db4bf3ab36086f80c1 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Fri, 2 Aug 2019 09:21:23 +0300 Subject: [PATCH 16/27] SIGINT not defined on windows --- tests/snippets/stdlib_signal.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/snippets/stdlib_signal.py b/tests/snippets/stdlib_signal.py index 933208a233..62c1355daa 100644 --- a/tests/snippets/stdlib_signal.py +++ b/tests/snippets/stdlib_signal.py @@ -3,8 +3,6 @@ import sys from testutils import assert_raises -assert_raises(TypeError, lambda: signal.signal(signal.SIGINT, 2)) - signals = [] def handler(signum, frame): @@ -13,6 +11,8 @@ def handler(signum, frame): # unix if "win" not in sys.platform: + assert_raises(TypeError, lambda: signal.signal(signal.SIGINT, 2)) + signal.signal(signal.SIGALRM, handler) assert signal.getsignal(signal.SIGALRM) is handler From 7cd5e895c7af85933ed1989574839e6aa892ac94 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Fri, 2 Aug 2019 09:39:56 +0300 Subject: [PATCH 17/27] Get SIG_IGN --- vm/src/stdlib/signal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index 5ff4f17605..80deda5584 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -65,7 +65,7 @@ fn signal( } let signal = vm.import("signal", &vm.ctx.new_tuple(vec![]), 0)?; let sig_dfl = vm.get_attribute(signal.clone(), "SIG_DFL")?; - let sig_ign = vm.get_attribute(signal, "SIG_DFL")?; + let sig_ign = vm.get_attribute(signal, "SIG_IGN")?; let signalnum = signalnum.as_bigint().to_i32().unwrap(); check_signals(vm); let mode = if handler.is(&sig_dfl) { From 52d204c6d3f760a65ba8afb0a530b323fba84753 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Fri, 2 Aug 2019 10:40:49 +0300 Subject: [PATCH 18/27] Fix clippy warnings --- Cargo.lock | 1 + vm/src/stdlib/signal.rs | 18 +++++++++--------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index af20cbf093..a8d85ef06d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1078,6 +1078,7 @@ dependencies = [ "unicode-xid 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "unicode_categories 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "unicode_names2 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", + "wtf8 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index 80deda5584..368a500e3a 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -26,18 +26,18 @@ extern "C" fn run_signal(signum: i32) { #[derive(Debug)] enum SigMode { - SigIgn, - SigDfl, - SigHandler, + Ign, + Dfl, + Handler, } #[cfg(unix)] fn os_set_signal(signalnum: i32, mode: SigMode, _vm: &VirtualMachine) { let signal_enum = signal::Signal::from_c_int(signalnum).unwrap(); let sig_handler = match mode { - SigMode::SigDfl => signal::SigHandler::SigDfl, - SigMode::SigIgn => signal::SigHandler::SigIgn, - SigMode::SigHandler => signal::SigHandler::Handler(run_signal), + SigMode::Dfl => signal::SigHandler::SigDfl, + SigMode::Ign => signal::SigHandler::SigIgn, + SigMode::Handler => signal::SigHandler::Handler(run_signal), }; let sig_action = signal::SigAction::new( sig_handler, @@ -69,11 +69,11 @@ fn signal( let signalnum = signalnum.as_bigint().to_i32().unwrap(); check_signals(vm); let mode = if handler.is(&sig_dfl) { - SigMode::SigDfl + SigMode::Dfl } else if handler.is(&sig_ign) { - SigMode::SigIgn + SigMode::Ign } else { - SigMode::SigHandler + SigMode::Handler }; os_set_signal(signalnum, mode, vm); let old_handler = vm.signal_handlers.borrow_mut().insert(signalnum, handler); From 60b5d9d12bd116327bd7116b10d8222eb0bafe3d Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Fri, 2 Aug 2019 10:44:22 +0300 Subject: [PATCH 19/27] Add empty check_signals on WASM --- vm/src/frame.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 0b67df1dcb..67ac625c50 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -19,12 +19,16 @@ use crate::pyobject::{ IdProtocol, ItemProtocol, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol, }; use crate::scope::{NameProtocol, Scope}; -#[cfg(not(target_arch = "wasm32"))] -use crate::stdlib::signal::check_signals; use crate::vm::VirtualMachine; use indexmap::IndexMap; use itertools::Itertools; +#[cfg(not(target_arch = "wasm32"))] +use crate::stdlib::signal::check_signals; + +#[cfg(target_arch = "wasm32")] +fn check_signals() {} + #[derive(Clone, Debug)] struct Block { /// The type of block. From 25b9f35de094f33bc3abec4a7ddd7d5d7c1d65be Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 3 Aug 2019 00:09:45 +0300 Subject: [PATCH 20/27] Fix more clippy warnings --- vm/src/stdlib/signal.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index 368a500e3a..a6618c1fc5 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -82,11 +82,7 @@ fn signal( fn getsignal(signalnum: PyIntRef, vm: &VirtualMachine) -> PyResult> { let signalnum = signalnum.as_bigint().to_i32().unwrap(); - Ok(vm - .signal_handlers - .borrow_mut() - .get(&signalnum) - .map(|x| x.clone())) + Ok(vm.signal_handlers.borrow_mut().get(&signalnum).cloned()) } #[cfg(unix)] @@ -100,6 +96,7 @@ fn alarm(time: PyIntRef, _vm: &VirtualMachine) -> u32 { prev_time.unwrap_or(0) } +#[allow(clippy::needless_range_loop)] pub fn check_signals(vm: &VirtualMachine) { for signum in 1..NSIG { let triggerd = unsafe { TRIGGERS[signum].swap(false, Ordering::Relaxed) }; From 3e07d61fcc43f3ef1e733a44a9ca4496ac81dfb3 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 3 Aug 2019 00:14:05 +0300 Subject: [PATCH 21/27] Add vm to WASM check_sginals --- vm/src/frame.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 67ac625c50..881defb5ae 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -27,7 +27,7 @@ use itertools::Itertools; use crate::stdlib::signal::check_signals; #[cfg(target_arch = "wasm32")] -fn check_signals() {} +fn check_signals(vm: &VirtualMachine) {} #[derive(Clone, Debug)] struct Block { From 075f2c9e74b583a980a09d34d10543623cff8a27 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 3 Aug 2019 10:13:56 +0300 Subject: [PATCH 22/27] Use libc directly to set signal --- vm/src/stdlib/signal.rs | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index a6618c1fc5..e9eda50125 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -13,6 +13,8 @@ use nix::sys::signal; #[cfg(unix)] use nix::unistd::alarm as sig_alarm; +use libc; + const NSIG: usize = 64; // We cannot use the NSIG const in the arr macro. This will fail compilation if NSIG is different. @@ -31,25 +33,18 @@ enum SigMode { Handler, } -#[cfg(unix)] -fn os_set_signal(signalnum: i32, mode: SigMode, _vm: &VirtualMachine) { - let signal_enum = signal::Signal::from_c_int(signalnum).unwrap(); +fn os_set_signal(signalnum: i32, mode: SigMode, vm: &VirtualMachine) -> PyResult<()> { let sig_handler = match mode { - SigMode::Dfl => signal::SigHandler::SigDfl, - SigMode::Ign => signal::SigHandler::SigIgn, - SigMode::Handler => signal::SigHandler::Handler(run_signal), + SigMode::Dfl => libc::SIG_DFL, + SigMode::Ign => libc::SIG_IGN, + SigMode::Handler => run_signal as libc::sighandler_t, }; - let sig_action = signal::SigAction::new( - sig_handler, - signal::SaFlags::empty(), - signal::SigSet::empty(), - ); - unsafe { signal::sigaction(signal_enum, &sig_action) }.unwrap(); -} - -#[cfg(not(unix))] -fn os_set_signal(_signalnum: i32, _mode: SigMode, _vm: &VirtualMachine) { - panic!("Not implemented"); + let old = unsafe { libc::signal(signalnum, sig_handler) }; + if old == libc::SIG_ERR { + Err(vm.new_os_error("Failed to set signal".to_string())) + } else { + Ok(()) + } } fn signal( @@ -75,7 +70,7 @@ fn signal( } else { SigMode::Handler }; - os_set_signal(signalnum, mode, vm); + os_set_signal(signalnum, mode, vm)?; let old_handler = vm.signal_handlers.borrow_mut().insert(signalnum, handler); Ok(old_handler) } From 23cba402a5ea03d4fb2f5f6f22d8e9e35c67645d Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 3 Aug 2019 10:21:51 +0300 Subject: [PATCH 23/27] Get signal numbers from libc --- vm/src/stdlib/signal.rs | 64 ++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index e9eda50125..f505be0bc3 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -8,8 +8,6 @@ use num_traits::cast::ToPrimitive; use arr_macro::arr; -#[cfg(unix)] -use nix::sys::signal; #[cfg(unix)] use nix::unistd::alarm as sig_alarm; @@ -123,6 +121,12 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { "getsignal" => ctx.new_rustfunc(getsignal), "SIG_DFL" => sig_dfl, "SIG_IGN" => sig_ign, + "SIGABRT" => ctx.new_int(libc::SIGABRT as u8), + "SIGFPE" => ctx.new_int(libc::SIGFPE as u8), + "SIGILL" => ctx.new_int(libc::SIGILL as u8), + "SIGINT" => ctx.new_int(libc::SIGINT as u8), + "SIGSEGV" => ctx.new_int(libc::SIGSEGV as u8), + "SIGTERM" => ctx.new_int(libc::SIGTERM as u8), }); extend_module_platform_specific(vm, module) } @@ -133,37 +137,31 @@ fn extend_module_platform_specific(vm: &VirtualMachine, module: PyObjectRef) -> extend_module!(vm, module, { "alarm" => ctx.new_rustfunc(alarm), - "SIGHUP" => ctx.new_int(signal::Signal::SIGHUP as u8), - "SIGINT" => ctx.new_int(signal::Signal::SIGINT as u8), - "SIGQUIT" => ctx.new_int(signal::Signal::SIGQUIT as u8), - "SIGILL" => ctx.new_int(signal::Signal::SIGILL as u8), - "SIGTRAP" => ctx.new_int(signal::Signal::SIGTRAP as u8), - "SIGABRT" => ctx.new_int(signal::Signal::SIGABRT as u8), - "SIGBUS" => ctx.new_int(signal::Signal::SIGBUS as u8), - "SIGFPE" => ctx.new_int(signal::Signal::SIGFPE as u8), - "SIGKILL" => ctx.new_int(signal::Signal::SIGKILL as u8), - "SIGUSR1" => ctx.new_int(signal::Signal::SIGUSR1 as u8), - "SIGSEGV" => ctx.new_int(signal::Signal::SIGSEGV as u8), - "SIGUSR2" => ctx.new_int(signal::Signal::SIGUSR2 as u8), - "SIGPIPE" => ctx.new_int(signal::Signal::SIGPIPE as u8), - "SIGALRM" => ctx.new_int(signal::Signal::SIGALRM as u8), - "SIGTERM" => ctx.new_int(signal::Signal::SIGTERM as u8), - "SIGSTKFLT" => ctx.new_int(signal::Signal::SIGSTKFLT as u8), - "SIGCHLD" => ctx.new_int(signal::Signal::SIGCHLD as u8), - "SIGCONT" => ctx.new_int(signal::Signal::SIGCONT as u8), - "SIGSTOP" => ctx.new_int(signal::Signal::SIGSTOP as u8), - "SIGTSTP" => ctx.new_int(signal::Signal::SIGTSTP as u8), - "SIGTTIN" => ctx.new_int(signal::Signal::SIGTTIN as u8), - "SIGTTOU" => ctx.new_int(signal::Signal::SIGTTOU as u8), - "SIGURG" => ctx.new_int(signal::Signal::SIGURG as u8), - "SIGXCPU" => ctx.new_int(signal::Signal::SIGXCPU as u8), - "SIGXFSZ" => ctx.new_int(signal::Signal::SIGXFSZ as u8), - "SIGVTALRM" => ctx.new_int(signal::Signal::SIGVTALRM as u8), - "SIGPROF" => ctx.new_int(signal::Signal::SIGPROF as u8), - "SIGWINCH" => ctx.new_int(signal::Signal::SIGWINCH as u8), - "SIGIO" => ctx.new_int(signal::Signal::SIGIO as u8), - "SIGPWR" => ctx.new_int(signal::Signal::SIGPWR as u8), - "SIGSYS" => ctx.new_int(signal::Signal::SIGSYS as u8), + "SIGHUP" => ctx.new_int(libc::SIGHUP as u8), + "SIGQUIT" => ctx.new_int(libc::SIGQUIT as u8), + "SIGTRAP" => ctx.new_int(libc::SIGTRAP as u8), + "SIGBUS" => ctx.new_int(libc::SIGBUS as u8), + "SIGKILL" => ctx.new_int(libc::SIGKILL as u8), + "SIGUSR1" => ctx.new_int(libc::SIGUSR1 as u8), + "SIGUSR2" => ctx.new_int(libc::SIGUSR2 as u8), + "SIGPIPE" => ctx.new_int(libc::SIGPIPE as u8), + "SIGALRM" => ctx.new_int(libc::SIGALRM as u8), + "SIGSTKFLT" => ctx.new_int(libc::SIGSTKFLT as u8), + "SIGCHLD" => ctx.new_int(libc::SIGCHLD as u8), + "SIGCONT" => ctx.new_int(libc::SIGCONT as u8), + "SIGSTOP" => ctx.new_int(libc::SIGSTOP as u8), + "SIGTSTP" => ctx.new_int(libc::SIGTSTP as u8), + "SIGTTIN" => ctx.new_int(libc::SIGTTIN as u8), + "SIGTTOU" => ctx.new_int(libc::SIGTTOU as u8), + "SIGURG" => ctx.new_int(libc::SIGURG as u8), + "SIGXCPU" => ctx.new_int(libc::SIGXCPU as u8), + "SIGXFSZ" => ctx.new_int(libc::SIGXFSZ as u8), + "SIGVTALRM" => ctx.new_int(libc::SIGVTALRM as u8), + "SIGPROF" => ctx.new_int(libc::SIGPROF as u8), + "SIGWINCH" => ctx.new_int(libc::SIGWINCH as u8), + "SIGIO" => ctx.new_int(libc::SIGIO as u8), + "SIGPWR" => ctx.new_int(libc::SIGPWR as u8), + "SIGSYS" => ctx.new_int(libc::SIGSYS as u8), }); module From f24c6daeb0de4e959faff9141d7cc437a00146bd Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 3 Aug 2019 10:37:56 +0300 Subject: [PATCH 24/27] Add some windows test to signal --- tests/snippets/stdlib_signal.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/snippets/stdlib_signal.py b/tests/snippets/stdlib_signal.py index 62c1355daa..eb4a25f90d 100644 --- a/tests/snippets/stdlib_signal.py +++ b/tests/snippets/stdlib_signal.py @@ -3,16 +3,24 @@ import sys from testutils import assert_raises +assert_raises(TypeError, lambda: signal.signal(signal.SIGINT, 2)) + signals = [] def handler(signum, frame): signals.append(signum) +signal.signal(signal.SIGILL, signal.SIG_IGN); +assert signal.getsignal(signal.SIGILL) is signal.SIG_IGN + +old_signal = signal.signal(signal.SIGILL, signal.SIG_DFL) +assert old_signal is signal.SIG_IGN +assert signal.getsignal(signal.SIGILL) is signal.SIG_DFL + + # unix if "win" not in sys.platform: - assert_raises(TypeError, lambda: signal.signal(signal.SIGINT, 2)) - signal.signal(signal.SIGALRM, handler) assert signal.getsignal(signal.SIGALRM) is handler From f600868a01f1a69b69ca601fc7b48cff3c50df39 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 3 Aug 2019 11:00:37 +0300 Subject: [PATCH 25/27] Define SIG_* on windows --- vm/src/stdlib/signal.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index f505be0bc3..9a989d8a0b 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -13,6 +13,16 @@ use nix::unistd::alarm as sig_alarm; use libc; +#[cfg(not(windows))] +use libc::{SIG_DFL, SIG_ERR, SIG_IGN}; + +#[cfg(windows)] +const SIG_DFL: libc::sighandler_t = 0; +#[cfg(windows)] +const SIG_IGN: libc::sighandler_t = 1; +#[cfg(windows)] +const SIG_ERR: libc::sighandler_t = !0; + const NSIG: usize = 64; // We cannot use the NSIG const in the arr macro. This will fail compilation if NSIG is different. @@ -33,12 +43,12 @@ enum SigMode { fn os_set_signal(signalnum: i32, mode: SigMode, vm: &VirtualMachine) -> PyResult<()> { let sig_handler = match mode { - SigMode::Dfl => libc::SIG_DFL, - SigMode::Ign => libc::SIG_IGN, + SigMode::Dfl => SIG_DFL, + SigMode::Ign => SIG_IGN, SigMode::Handler => run_signal as libc::sighandler_t, }; let old = unsafe { libc::signal(signalnum, sig_handler) }; - if old == libc::SIG_ERR { + if old == SIG_ERR { Err(vm.new_os_error("Failed to set signal".to_string())) } else { Ok(()) From c5486a63085a2dd413724ba4594d9a34092913b6 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 3 Aug 2019 12:26:08 +0300 Subject: [PATCH 26/27] Remove stub check_signals in WASM --- vm/src/frame.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 881defb5ae..b9ba09905c 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -26,9 +26,6 @@ use itertools::Itertools; #[cfg(not(target_arch = "wasm32"))] use crate::stdlib::signal::check_signals; -#[cfg(target_arch = "wasm32")] -fn check_signals(vm: &VirtualMachine) {} - #[derive(Clone, Debug)] struct Block { /// The type of block. @@ -169,7 +166,10 @@ impl Frame { /// Execute a single instruction. #[allow(clippy::cognitive_complexity)] fn execute_instruction(&self, vm: &VirtualMachine) -> FrameResult { - check_signals(vm); + #[cfg(not(target_arch = "wasm32"))] + { + check_signals(vm); + } let instruction = self.fetch_instruction(); flame_guard!(format!("Frame::execute_instruction({:?})", instruction)); From 5e5d46d373c7c9367b68e064d7d43a84aaa1e802 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 3 Aug 2019 12:31:55 +0300 Subject: [PATCH 27/27] Remove os_set_signal --- vm/src/stdlib/signal.rs | 40 +++++++++++----------------------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index 9a989d8a0b..cabeb12f7f 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -34,27 +34,6 @@ extern "C" fn run_signal(signum: i32) { } } -#[derive(Debug)] -enum SigMode { - Ign, - Dfl, - Handler, -} - -fn os_set_signal(signalnum: i32, mode: SigMode, vm: &VirtualMachine) -> PyResult<()> { - let sig_handler = match mode { - SigMode::Dfl => SIG_DFL, - SigMode::Ign => SIG_IGN, - SigMode::Handler => run_signal as libc::sighandler_t, - }; - let old = unsafe { libc::signal(signalnum, sig_handler) }; - if old == SIG_ERR { - Err(vm.new_os_error("Failed to set signal".to_string())) - } else { - Ok(()) - } -} - fn signal( signalnum: PyIntRef, handler: PyObjectRef, @@ -66,19 +45,22 @@ fn signal( { return Err(vm.new_type_error("Hanlder must be callable".to_string())); } - let signal = vm.import("signal", &vm.ctx.new_tuple(vec![]), 0)?; - let sig_dfl = vm.get_attribute(signal.clone(), "SIG_DFL")?; - let sig_ign = vm.get_attribute(signal, "SIG_IGN")?; + let signal_module = vm.import("signal", &vm.ctx.new_tuple(vec![]), 0)?; + let sig_dfl = vm.get_attribute(signal_module.clone(), "SIG_DFL")?; + let sig_ign = vm.get_attribute(signal_module, "SIG_IGN")?; let signalnum = signalnum.as_bigint().to_i32().unwrap(); check_signals(vm); - let mode = if handler.is(&sig_dfl) { - SigMode::Dfl + let sig_handler = if handler.is(&sig_dfl) { + SIG_DFL } else if handler.is(&sig_ign) { - SigMode::Ign + SIG_IGN } else { - SigMode::Handler + run_signal as libc::sighandler_t }; - os_set_signal(signalnum, mode, vm)?; + let old = unsafe { libc::signal(signalnum, sig_handler) }; + if old == SIG_ERR { + return Err(vm.new_os_error("Failed to set signal".to_string())); + } let old_handler = vm.signal_handlers.borrow_mut().insert(signalnum, handler); Ok(old_handler) }