mirror of
https://github.com/RustPython/RustPython.git
synced 2026-10-28 18:59:04 +08:00
Drop the handler-table borrow before running a signal handler (#8582)
trigger_signals borrowed the handler table for the whole dispatch loop
and the borrow was still alive while the Python handler ran. A handler
that disarms itself, which is the ordinary way to write one, reached for
the same cell:
>>> import signal
>>> def h(signum, frame): signal.signal(signal.SIGUSR1, signal.SIG_IGN)
...
>>> signal.signal(signal.SIGUSR1, h)
>>> signal.raise_signal(signal.SIGUSR1)
thread 'main' panicked at crates/vm/src/stdlib/_signal.rs:255:43:
RefCell already borrowed
The handler now comes out of the table one signal at a time, and the
borrow ends before it runs. Reading inside the loop rather than once up
front also keeps the table current, so a handler that arms or disarms
another signal is obeyed for the rest of the pass.
Assisted-by: Claude Code:claude-opus-5
This commit is contained in:
1 parent
e71a955c99
commit
d1e80365d6
3 files changed
+46
-10
No files matched your search
Vendored
-6
@@ -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
|
||||
|
||||
|
||||
+10
-4
@@ -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)?;
|
||||
|
||||
@@ -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)
|
||||
Reference in new issue
Block a user