library + trace: rewrite panic sites as explicit match+panic

Apply the same explicit match+panic shape to the library layer (channel,
gen_server, gen_statem). Extend it to the smarm-trace-gated code that the
default `cargo clippy --lib` does not see: the GLOBAL lock-poison sites in
trace.rs and the current_pid sites inside te!() in channel.rs. Keep the
current_pid match inside the te!() argument so non-trace builds evaluate
nothing extra on the recv-wake hot path. const-init the trace thread-local.
This commit is contained in:
smarm-agent
2026-06-20 17:47:39 +00:00
parent a875fa8285
commit 33177a0c48
4 changed files with 132 additions and 43 deletions
+54 -18
View File
@@ -176,8 +176,10 @@ impl<T> Receiver<T> {
if g.senders == 0 { if g.senders == 0 {
return Err(RecvError); return Err(RecvError);
} }
let me = crate::actor::current_pid() let me = match crate::actor::current_pid() {
.expect("recv() called outside an actor"); Some(me) => me,
None => panic!("smarm: recv() called outside an actor"),
};
debug_assert!( debug_assert!(
g.parked_receiver.is_none_or(|(p, _)| p == me), g.parked_receiver.is_none_or(|(p, _)| p == me),
"channel has more than one receiver" "channel has more than one receiver"
@@ -191,7 +193,10 @@ impl<T> Receiver<T> {
// Release the lock before parking — the unparker will need it. // Release the lock before parking — the unparker will need it.
crate::scheduler::park_current(); crate::scheduler::park_current();
// Woken up — record it before looping to check the queue. // Woken up — record it before looping to check the queue.
crate::te!(crate::trace::Event::RecvWake(crate::actor::current_pid().unwrap())); crate::te!(crate::trace::Event::RecvWake(match crate::actor::current_pid() {
Some(p) => p,
None => panic!("smarm: RecvWake outside an actor (core corrupt)"),
}));
} }
} }
@@ -222,8 +227,10 @@ impl<T> Receiver<T> {
where where
T: Send + 'static, T: Send + 'static,
{ {
let me = crate::actor::current_pid() let me = match crate::actor::current_pid() {
.expect("recv_timeout() called outside an actor"); Some(me) => me,
None => panic!("smarm: recv_timeout() called outside an actor"),
};
// Fast path + wait registration, one critical section. // Fast path + wait registration, one critical section.
let epoch; let epoch;
@@ -254,7 +261,10 @@ impl<T> Receiver<T> {
crate::scheduler::insert_wait_timer(deadline, me, target, epoch); crate::scheduler::insert_wait_timer(deadline, me, target, epoch);
crate::scheduler::park_current(); crate::scheduler::park_current();
crate::te!(crate::trace::Event::RecvWake(crate::actor::current_pid().unwrap())); crate::te!(crate::trace::Event::RecvWake(match crate::actor::current_pid() {
Some(p) => p,
None => panic!("smarm: RecvWake outside an actor (core corrupt)"),
}));
let mut g = self.inner.lock(); let mut g = self.inner.lock();
if let Some(v) = g.queue.pop_front() { if let Some(v) = g.queue.pop_front() {
crate::preempt::note_message_received(); crate::preempt::note_message_received();
@@ -283,17 +293,23 @@ impl<T> Receiver<T> {
loop { loop {
{ {
let mut g = self.inner.lock(); let mut g = self.inner.lock();
if let Some(i) = g.queue.iter().position(|v| pred(v)) { if let Some(i) = g.queue.iter().position(&pred) {
// position() found it, so remove() returns Some. // position() found it, so remove() returns Some.
crate::preempt::note_message_received(); crate::preempt::note_message_received();
return Ok(g.queue.remove(i).unwrap()); let v = match g.queue.remove(i) {
Some(v) => v,
None => panic!("smarm: channel queue.remove after position (logic bug)"),
};
return Ok(v);
} }
if g.senders == 0 { if g.senders == 0 {
// Closed and nothing queued can ever match. // Closed and nothing queued can ever match.
return Err(RecvError); return Err(RecvError);
} }
let me = crate::actor::current_pid() let me = match crate::actor::current_pid() {
.expect("recv_match() called outside an actor"); Some(me) => me,
None => panic!("smarm: recv_match() called outside an actor"),
};
debug_assert!( debug_assert!(
g.parked_receiver.is_none_or(|(p, _)| p == me), g.parked_receiver.is_none_or(|(p, _)| p == me),
"channel has more than one receiver" "channel has more than one receiver"
@@ -303,7 +319,10 @@ impl<T> Receiver<T> {
} }
// Release the lock before parking — the unparker will need it. // Release the lock before parking — the unparker will need it.
crate::scheduler::park_current(); crate::scheduler::park_current();
crate::te!(crate::trace::Event::RecvWake(crate::actor::current_pid().unwrap())); crate::te!(crate::trace::Event::RecvWake(match crate::actor::current_pid() {
Some(p) => p,
None => panic!("smarm: RecvWake outside an actor (core corrupt)"),
}));
} }
} }
@@ -316,9 +335,13 @@ impl<T> Receiver<T> {
F: Fn(&T) -> bool, F: Fn(&T) -> bool,
{ {
let mut g = self.inner.lock(); let mut g = self.inner.lock();
if let Some(i) = g.queue.iter().position(|v| pred(v)) { if let Some(i) = g.queue.iter().position(&pred) {
crate::preempt::note_message_received(); crate::preempt::note_message_received();
return Ok(Some(g.queue.remove(i).unwrap())); let v = match g.queue.remove(i) {
Some(v) => v,
None => panic!("smarm: channel queue.remove after position (logic bug)"),
};
return Ok(Some(v));
} }
if g.senders == 0 { if g.senders == 0 {
return Err(RecvError); return Err(RecvError);
@@ -466,7 +489,10 @@ impl<T> Selectable for Receiver<T> {
/// see [`try_select`] for the fallible form; channel-only selects cannot /// see [`try_select`] for the fallible form; channel-only selects cannot
/// fail). /// fail).
pub fn select(arms: &[&dyn Selectable]) -> usize { pub fn select(arms: &[&dyn Selectable]) -> usize {
try_select(arms).expect("select(): fd arm failed to register (use try_select)") match try_select(arms) {
Ok(i) => i,
Err(e) => panic!("smarm: select() fd arm failed to register (use try_select): {e}"),
}
} }
/// [`select`], fallible: `Err` when an arm fails to register (only fd /// [`select`], fallible: `Err` when an arm fails to register (only fd
@@ -476,7 +502,10 @@ pub fn select(arms: &[&dyn Selectable]) -> usize {
/// one has been unregistered. /// one has been unregistered.
pub fn try_select(arms: &[&dyn Selectable]) -> std::io::Result<usize> { pub fn try_select(arms: &[&dyn Selectable]) -> std::io::Result<usize> {
assert!(!arms.is_empty(), "select() on an empty arm list"); assert!(!arms.is_empty(), "select() on an empty arm list");
let me = crate::actor::current_pid().expect("select() called outside an actor"); let me = match crate::actor::current_pid() {
Some(me) => me,
None => panic!("smarm: select() called outside an actor"),
};
loop { loop {
let epoch = crate::scheduler::begin_wait(); let epoch = crate::scheduler::begin_wait();
if let Some(i) = register_arms(me, epoch, arms)? { if let Some(i) = register_arms(me, epoch, arms)? {
@@ -626,8 +655,12 @@ pub fn select_timeout(
arms: &[&dyn Selectable], arms: &[&dyn Selectable],
timeout: std::time::Duration, timeout: std::time::Duration,
) -> Option<usize> { ) -> Option<usize> {
try_select_timeout(arms, timeout) match try_select_timeout(arms, timeout) {
.expect("select_timeout(): fd arm failed to register (use try_select_timeout)") Ok(r) => r,
Err(e) => panic!(
"smarm: select_timeout() fd arm failed to register (use try_select_timeout): {e}"
),
}
} }
/// [`select_timeout`], fallible: `Err` when an arm fails to register /// [`select_timeout`], fallible: `Err` when an arm fails to register
@@ -638,7 +671,10 @@ pub fn try_select_timeout(
timeout: std::time::Duration, timeout: std::time::Duration,
) -> std::io::Result<Option<usize>> { ) -> std::io::Result<Option<usize>> {
assert!(!arms.is_empty(), "select_timeout() on an empty arm list"); assert!(!arms.is_empty(), "select_timeout() on an empty arm list");
let me = crate::actor::current_pid().expect("select_timeout() called outside an actor"); let me = match crate::actor::current_pid() {
Some(me) => me,
None => panic!("smarm: select_timeout() called outside an actor"),
};
let epoch = crate::scheduler::begin_wait(); let epoch = crate::scheduler::begin_wait();
if let Some(i) = register_arms(me, epoch, arms)? { if let Some(i) = register_arms(me, epoch, arms)? {
return Ok(Some(i)); // ready now: the timer was never armed return Ok(Some(i)); // ready now: the timer was never armed
+31 -12
View File
@@ -528,7 +528,10 @@ impl<G: GenServer> TimerHandle<G> {
/// [`TimerId`] you can pass to `cancel`. Timer fires arrive before info and /// [`TimerId`] you can pass to `cancel`. Timer fires arrive before info and
/// inbox messages. /// inbox messages.
pub fn arm_after(&self, after: Duration, msg: G::Timer) -> TimerId { pub fn arm_after(&self, after: Duration, msg: G::Timer) -> TimerId {
let mut reg = self.reg.lock().unwrap(); let mut reg = match self.reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_server reg lock poisoned (core corrupt): {e}"),
};
let local = reg.mint(); let local = reg.mint();
// The fire thunk carries the local id so the loop can retire the entry // The fire thunk carries the local id so the loop can retire the entry
// on dispatch; `msg` is moved in (no `Clone` needed for one-shots). // on dispatch; `msg` is moved in (no `Clone` needed for one-shots).
@@ -552,7 +555,10 @@ impl<G: GenServer> TimerHandle<G> {
where where
G::Timer: Clone, G::Timer: Clone,
{ {
let mut reg = self.reg.lock().unwrap(); let mut reg = match self.reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_server reg lock poisoned (core corrupt): {e}"),
};
let local = reg.mint(); let local = reg.mint();
// A live periodic must keep the system arm open so the next tick can be // A live periodic must keep the system arm open so the next tick can be
// re-armed; the first periodic installs the loop's re-arm sender. // re-armed; the first periodic installs the loop's re-arm sender.
@@ -577,7 +583,10 @@ impl<G: GenServer> TimerHandle<G> {
/// stops re-arming: the entry is removed so the loop will not schedule /// stops re-arming: the entry is removed so the loop will not schedule
/// another tick even if the pending one already escaped onto the channel. /// another tick even if the pending one already escaped onto the channel.
pub fn cancel(&self, id: TimerId) -> bool { pub fn cancel(&self, id: TimerId) -> bool {
let mut reg = self.reg.lock().unwrap(); let mut reg = match self.reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_server reg lock poisoned (core corrupt): {e}"),
};
if let Some(sub) = reg.oneshots.remove(&id) { if let Some(sub) = reg.oneshots.remove(&id) {
return cancel_timer(sub); return cancel_timer(sub);
} }
@@ -587,7 +596,7 @@ impl<G: GenServer> TimerHandle<G> {
reg.rearm_tx = None; reg.rearm_tx = None;
} }
debug_assert!( debug_assert!(
reg.rearm_tx.is_some() == !reg.periodics.is_empty(), reg.rearm_tx.is_some() != reg.periodics.is_empty(),
"rearm_tx must be Some iff periodics is non-empty" "rearm_tx must be Some iff periodics is non-empty"
); );
return beat; return beat;
@@ -846,7 +855,10 @@ fn server_loop<G: GenServer>(
impl<G: GenServer> Drop for Terminate<G> { impl<G: GenServer> Drop for Terminate<G> {
fn drop(&mut self) { fn drop(&mut self) {
{ {
let mut reg = self.1.lock().unwrap(); let mut reg = match self.1.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_server reg lock poisoned (core corrupt): {e}"),
};
for (_, sub) in reg.oneshots.drain() { for (_, sub) in reg.oneshots.drain() {
cancel_timer(sub); cancel_timer(sub);
} }
@@ -995,7 +1007,10 @@ fn server_loop<G: GenServer>(
// The one-shot fired: retire its registry entry so the // The one-shot fired: retire its registry entry so the
// live set tracks only still-pending timers, then // live set tracks only still-pending timers, then
// dispatch. // dispatch.
reg.lock().unwrap().oneshots.remove(&id); match reg.lock() {
Ok(mut g) => { g.oneshots.remove(&id); }
Err(e) => panic!("smarm: gen_server reg lock poisoned (core corrupt): {e}"),
}
guard.0.handle_timer(msg); guard.0.handle_timer(msg);
reset_idle(&mut idle_deadline); reset_idle(&mut idle_deadline);
} }
@@ -1006,16 +1021,20 @@ fn server_loop<G: GenServer>(
// so the period is measured from fire-handling and a // so the period is measured from fire-handling and a
// handler that cancels stops the just-armed instance. // handler that cancels stops the just-armed instance.
let msg = { let msg = {
let mut g = reg.lock().unwrap(); let mut g = match reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_server reg lock poisoned (core corrupt): {e}"),
};
let r = &mut *g; let r = &mut *g;
if let Some(p) = r.periodics.get_mut(&id) { if let Some(p) = r.periodics.get_mut(&id) {
let every = p.every; let every = p.every;
let msg = (p.make)(); let msg = (p.make)();
let tx = r let tx = match r.rearm_tx.as_ref() {
.rearm_tx Some(tx) => tx.clone(),
.as_ref() None => panic!(
.expect("a live periodic keeps rearm_tx Some") "smarm: live periodic without rearm_tx (logic bug)"
.clone(); ),
};
p.live = send_after_to(every, tx, Sys::Tick(id)); p.live = send_after_to(every, tx, Sys::Tick(id));
Some(msg) Some(msg)
} else { } else {
+28 -7
View File
@@ -257,7 +257,10 @@ impl<Ev> Cx<Ev> {
/// state. Only one is ever pending — arming again cancels and replaces the /// state. Only one is ever pending — arming again cancels and replaces the
/// previous one. /// previous one.
pub fn state_timeout(&mut self, after: Duration) { pub fn state_timeout(&mut self, after: Duration) {
let mut reg = self.reg.lock().unwrap(); let mut reg = match self.reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_statem reg lock poisoned (core corrupt): {e}"),
};
if let Some((_, old_sub)) = reg.state.take() { if let Some((_, old_sub)) = reg.state.take() {
cancel_timer(old_sub); cancel_timer(old_sub);
} }
@@ -271,7 +274,10 @@ impl<Ev> Cx<Ev> {
/// `name`. Arming the same `name` again cancels and replaces the pending one /// `name`. Arming the same `name` again cancels and replaces the pending one
/// for that name. /// for that name.
pub fn timeout(&mut self, name: &'static str, after: Duration) { pub fn timeout(&mut self, name: &'static str, after: Duration) {
let mut reg = self.reg.lock().unwrap(); let mut reg = match self.reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_statem reg lock poisoned (core corrupt): {e}"),
};
if let Some((_, old_sub)) = reg.named.remove(name) { if let Some((_, old_sub)) = reg.named.remove(name) {
cancel_timer(old_sub); cancel_timer(old_sub);
} }
@@ -283,7 +289,10 @@ impl<Ev> Cx<Ev> {
/// Cancel the named timeout `name` if pending. Returns `true` if the cancel /// Cancel the named timeout `name` if pending. Returns `true` if the cancel
/// beat the fire, `false` if it had already fired / was never armed. /// beat the fire, `false` if it had already fired / was never armed.
pub fn cancel_timeout(&mut self, name: &'static str) -> bool { pub fn cancel_timeout(&mut self, name: &'static str) -> bool {
let mut reg = self.reg.lock().unwrap(); let mut reg = match self.reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_statem reg lock poisoned (core corrupt): {e}"),
};
match reg.named.remove(name) { match reg.named.remove(name) {
Some((_, sub)) => cancel_timer(sub), Some((_, sub)) => cancel_timer(sub),
None => false, None => false,
@@ -294,7 +303,10 @@ impl<Ev> Cx<Ev> {
/// beat the fire. Rarely needed by hand (the loop auto-resets on /// beat the fire. Rarely needed by hand (the loop auto-resets on
/// transition); exposed for a handler that wants to disarm within a state. /// transition); exposed for a handler that wants to disarm within a state.
pub fn cancel_state_timeout(&mut self) -> bool { pub fn cancel_state_timeout(&mut self) -> bool {
let mut reg = self.reg.lock().unwrap(); let mut reg = match self.reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_statem reg lock poisoned (core corrupt): {e}"),
};
match reg.state.take() { match reg.state.take() {
Some((_, sub)) => cancel_timer(sub), Some((_, sub)) => cancel_timer(sub),
None => false, None => false,
@@ -311,7 +323,10 @@ impl<Ev> Cx<Ev> {
/// `enter` if it wants one. /// `enter` if it wants one.
#[doc(hidden)] #[doc(hidden)]
pub fn __reset_state_timeout(&mut self) { pub fn __reset_state_timeout(&mut self) {
let mut reg = self.reg.lock().unwrap(); let mut reg = match self.reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_statem reg lock poisoned (core corrupt): {e}"),
};
if let Some((_, sub)) = reg.state.take() { if let Some((_, sub)) = reg.state.take() {
cancel_timer(sub); cancel_timer(sub);
} }
@@ -458,7 +473,10 @@ fn statem_loop<M: Machine>(rx: Receiver<M::Ev>, mut machine: M) {
// escaped onto the channel. A stale fire is dropped. // escaped onto the channel. A stale fire is dropped.
let ev = match fire { let ev = match fire {
Sys::StateTimeout(local) => { Sys::StateTimeout(local) => {
let mut t = reg.lock().unwrap(); let mut t = match reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_statem reg lock poisoned (core corrupt): {e}"),
};
match t.state { match t.state {
Some((live, _)) if live == local => { Some((live, _)) if live == local => {
t.state = None; // retire: it has now fired t.state = None; // retire: it has now fired
@@ -468,7 +486,10 @@ fn statem_loop<M: Machine>(rx: Receiver<M::Ev>, mut machine: M) {
} }
} }
Sys::Timeout(name, local) => { Sys::Timeout(name, local) => {
let mut t = reg.lock().unwrap(); let mut t = match reg.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: gen_statem reg lock poisoned (core corrupt): {e}"),
};
match t.named.get(name) { match t.named.get(name) {
Some(&(live, _)) if live == local => { Some(&(live, _)) if live == local => {
t.named.remove(name); // retire on fire t.named.remove(name); // retire on fire
+19 -6
View File
@@ -101,7 +101,7 @@ mod inner {
thread_local! { thread_local! {
static LOCAL_STATE: std::cell::RefCell<Option<LocalState>> = static LOCAL_STATE: std::cell::RefCell<Option<LocalState>> =
std::cell::RefCell::new(None); const { std::cell::RefCell::new(None) };
} }
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
@@ -115,14 +115,20 @@ mod inner {
let (tx, rx) = mpsc::channel::<Msg>(); let (tx, rx) = mpsc::channel::<Msg>();
let start = Instant::now(); let start = Instant::now();
*GLOBAL.lock().unwrap() = Some(Global { sender: tx, start }); match GLOBAL.lock() {
Ok(mut g) => *g = Some(Global { sender: tx, start }),
Err(e) => panic!("smarm: trace lock poisoned (core corrupt): {e}"),
}
// Drain thread: owns the Receiver, writes to disk. // Drain thread: owns the Receiver, writes to disk.
let path_for_thread = path.clone(); let path_for_thread = path.clone();
std::thread::Builder::new() match std::thread::Builder::new()
.name("smarm-trace-drain".into()) .name("smarm-trace-drain".into())
.spawn(move || drain_thread(rx, &path_for_thread)) .spawn(move || drain_thread(rx, &path_for_thread))
.expect("failed to spawn trace drain thread"); {
Ok(_) => {}
Err(e) => panic!("smarm: failed to spawn trace drain thread: {e}"),
}
eprintln!("[smarm-trace] writing to {}", path); eprintln!("[smarm-trace] writing to {}", path);
} }
@@ -133,7 +139,10 @@ mod inner {
// Drop the global sender so the drain thread's recv() returns Err // Drop the global sender so the drain thread's recv() returns Err
// after the Flush sentinel, signalling clean shutdown. // after the Flush sentinel, signalling clean shutdown.
let sender = { let sender = {
let mut g = GLOBAL.lock().unwrap(); let mut g = match GLOBAL.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: trace lock poisoned (core corrupt): {e}"),
};
g.take().map(|g| g.sender) g.take().map(|g| g.sender)
}; };
if let Some(tx) = sender { if let Some(tx) = sender {
@@ -162,7 +171,11 @@ mod inner {
let mut opt = cell.borrow_mut(); let mut opt = cell.borrow_mut();
// Lazily initialise: one mutex hit per thread, ever. // Lazily initialise: one mutex hit per thread, ever.
if opt.is_none() { if opt.is_none() {
if let Some(g) = GLOBAL.lock().unwrap().as_ref() { let guard = match GLOBAL.lock() {
Ok(g) => g,
Err(e) => panic!("smarm: trace lock poisoned (core corrupt): {e}"),
};
if let Some(g) = guard.as_ref() {
let tx = g.sender.clone(); let tx = g.sender.clone();
*opt = Some(LocalState { tx, start: g.start }); *opt = Some(LocalState { tx, start: g.start });
} }