diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 99ab6c7ba..b023b1c5d 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -5053,13 +5053,11 @@ class SignalsTest(unittest.TestCase): if e.errno != errno.EBADF: raise - @unittest.skip("TODO: RUSTPYTHON; thread 'main' (103833) panicked at crates/vm/src/stdlib/signal.rs:233:43: RefCell already borrowed") @requires_alarm @support.requires_resource('walltime') def test_interrupted_write_retry_buffered(self): self.check_interrupted_write_retry(b"x", mode="wb") - @unittest.skip("TODO: RUSTPYTHON; thread 'main' (103833) panicked at crates/vm/src/stdlib/signal.rs:233:43: RefCell already borrowed") @requires_alarm @support.requires_resource('walltime') def test_interrupted_write_retry_text(self): @@ -5069,10 +5067,6 @@ class SignalsTest(unittest.TestCase): class CSignalsTest(SignalsTest): io = io - @unittest.skip("TODO: RUSTPYTHON; thread 'main' (103833) panicked at crates/vm/src/stdlib/signal.rs:233:43: RefCell already borrowed") - def test_interrupted_read_retry_buffered(self): - return super().test_interrupted_read_retry_buffered() - class PySignalsTest(SignalsTest): io = pyio diff --git a/crates/vm/src/signal.rs b/crates/vm/src/signal.rs index 16a097ea6..d1ddd8c3c 100644 --- a/crates/vm/src/signal.rs +++ b/crates/vm/src/signal.rs @@ -88,17 +88,23 @@ fn trigger_signals(vm: &VirtualMachine) -> PyResult<()> { let signal_handlers = vm .signal_handlers .get() - .expect("should never fail since we check above") - .borrow(); + .expect("should never fail since we check above"); for (signum, trigger) in TRIGGERS.iter().enumerate().skip(1) { let triggered = trigger.swap(false, Ordering::Relaxed); + if !triggered { + continue; + } // SAFETY: TRIGGERS has the same length as the signal_handlers let signum = unsafe { SignalNum::new_unchecked(signum as i32) }; - if triggered - && let Some(handler) = &signal_handlers[signum] + // Read the handler out and drop the borrow before running it. A + // handler is free to call signal.signal(), which takes the same cell + // mutably, and a live read borrow turns that into a panic. + let handler = signal_handlers.borrow()[signum].clone(); + + if let Some(handler) = handler && let Some(callable) = handler.to_callable() { callable.invoke((signum.as_i32(), vm.ctx.none()), vm)?; diff --git a/extra_tests/snippets/stdlib_signal.py b/extra_tests/snippets/stdlib_signal.py index 9d0b13bbd..50a025a06 100644 --- a/extra_tests/snippets/stdlib_signal.py +++ b/extra_tests/snippets/stdlib_signal.py @@ -41,3 +41,39 @@ if "win" not in sys.platform: time.sleep(2.0) assert signals == [signal.SIGALRM, signal.SIGALRM] + + # A handler may call signal.signal(), and the usual reason is to disarm + # itself. Reading the handler table while the handler runs used to be a + # crash rather than a rearm. + rearmed = [] + + def rearm(signum, frame): + rearmed.append(signum) + signal.signal(signal.SIGALRM, signal.SIG_IGN) + + signal.signal(signal.SIGALRM, rearm) + signal.raise_signal(signal.SIGALRM) + assert rearmed == [signal.SIGALRM], rearmed + assert signal.getsignal(signal.SIGALRM) is signal.SIG_IGN + + # The same goes for arming a different signal from inside a handler. + armed = [] + + def target(signum, frame): + armed.append("target") + + def arm_other(signum, frame): + armed.append("arm_other") + signal.signal(signal.SIGUSR2, target) + + signal.signal(signal.SIGUSR1, arm_other) + signal.raise_signal(signal.SIGUSR1) + assert armed == ["arm_other"], armed + assert signal.getsignal(signal.SIGUSR2) is target + + signal.raise_signal(signal.SIGUSR2) + assert armed == ["arm_other", "target"], armed + + signal.signal(signal.SIGALRM, signal.SIG_DFL) + signal.signal(signal.SIGUSR1, signal.SIG_DFL) + signal.signal(signal.SIGUSR2, signal.SIG_DFL)