FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Drop the handler-table borrow before running a signal handler (#8582) · RustPython/RustPython@0dfc93a · GitHub

/ RustPython Public

Commit 0dfc93a

Browse files
luantaraschi
authored
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
1 parent cc1e55e commit 0dfc93a

3 files changed

Lines changed: 46 additions & 10 deletions

File tree

‎Lib/test/test_io.py‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5053,13 +5053,11 @@ def alarm2(sig, frame):
50535053
if e.errno != errno.EBADF:
50545054
raise
50555055

5056-
@unittest.skip("TODO: RUSTPYTHON; thread 'main' (103833) panicked at crates/vm/src/stdlib/signal.rs:233:43: RefCell already borrowed")
50575056
@requires_alarm
50585057
@support.requires_resource('walltime')
50595058
def test_interrupted_write_retry_buffered(self):
50605059
self.check_interrupted_write_retry(b"x", mode="wb")
50615060

5062-
@unittest.skip("TODO: RUSTPYTHON; thread 'main' (103833) panicked at crates/vm/src/stdlib/signal.rs:233:43: RefCell already borrowed")
50635061
@requires_alarm
50645062
@support.requires_resource('walltime')
50655063
def test_interrupted_write_retry_text(self):
@@ -5069,10 +5067,6 @@ def test_interrupted_write_retry_text(self):
50695067
class CSignalsTest(SignalsTest):
50705068
io = io
50715069

5072-
@unittest.skip("TODO: RUSTPYTHON; thread 'main' (103833) panicked at crates/vm/src/stdlib/signal.rs:233:43: RefCell already borrowed")
5073-
def test_interrupted_read_retry_buffered(self):
5074-
return super().test_interrupted_read_retry_buffered()
5075-
50765070
class PySignalsTest(SignalsTest):
50775071
io = pyio
50785072

‎crates/vm/src/signal.rs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,17 +88,23 @@ fn trigger_signals(vm: &VirtualMachine) -> PyResult<()> {
8888
let signal_handlers = vm
8989
.signal_handlers
9090
.get()
91-
.expect("should never fail since we check above")
92-
.borrow();
91+
.expect("should never fail since we check above");
9392

9493
for (signum, trigger) in TRIGGERS.iter().enumerate().skip(1) {
9594
let triggered = trigger.swap(false, Ordering::Relaxed);
95+
if !triggered {
96+
continue;
97+
}
9698

9799
// SAFETY: TRIGGERS has the same length as the signal_handlers
98100
let signum = unsafe { SignalNum::new_unchecked(signum as i32) };
99101

100-
if triggered
101-
&& let Some(handler) = &signal_handlers[signum]
102+
// Read the handler out and drop the borrow before running it. A
103+
// handler is free to call signal.signal(), which takes the same cell
104+
// mutably, and a live read borrow turns that into a panic.
105+
let handler = signal_handlers.borrow()[signum].clone();
106+
107+
if let Some(handler) = handler
102108
&& let Some(callable) = handler.to_callable()
103109
{
104110
callable.invoke((signum.as_i32(), vm.ctx.none()), vm)?;

‎extra_tests/snippets/stdlib_signal.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,39 @@ def handler(signum, frame):
4141
time.sleep(2.0)
4242

4343
assert signals == [signal.SIGALRM, signal.SIGALRM]
44+
45+
# A handler may call signal.signal(), and the usual reason is to disarm
46+
# itself. Reading the handler table while the handler runs used to be a
47+
# crash rather than a rearm.
48+
rearmed = []
49+
50+
def rearm(signum, frame):
51+
rearmed.append(signum)
52+
signal.signal(signal.SIGALRM, signal.SIG_IGN)
53+
54+
signal.signal(signal.SIGALRM, rearm)
55+
signal.raise_signal(signal.SIGALRM)
56+
assert rearmed == [signal.SIGALRM], rearmed
57+
assert signal.getsignal(signal.SIGALRM) is signal.SIG_IGN
58+
59+
# The same goes for arming a different signal from inside a handler.
60+
armed = []
61+
62+
def target(signum, frame):
63+
armed.append("target")
64+
65+
def arm_other(signum, frame):
66+
armed.append("arm_other")
67+
signal.signal(signal.SIGUSR2, target)
68+
69+
signal.signal(signal.SIGUSR1, arm_other)
70+
signal.raise_signal(signal.SIGUSR1)
71+
assert armed == ["arm_other"], armed
72+
assert signal.getsignal(signal.SIGUSR2) is target
73+
74+
signal.raise_signal(signal.SIGUSR2)
75+
assert armed == ["arm_other", "target"], armed
76+
77+
signal.signal(signal.SIGALRM, signal.SIG_DFL)
78+
signal.signal(signal.SIGUSR1, signal.SIG_DFL)
79+
signal.signal(signal.SIGUSR2, signal.SIG_DFL)

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL