From b3af1659b20ab00d6aca3f85a9ccfa30d1e8592b Mon Sep 17 00:00:00 2001 From: Zhiyao Ma Date: Wed, 9 Oct 2024 14:18:46 -0400 Subject: [PATCH] Panic the task or ISR upon trying to block when it must not. --- src/sync/mailbox.rs | 16 +++++++++++++--- src/sync/wait_queue.rs | 28 +++++++++++++++++++++------- src/time/mod.rs | 7 +++++++ src/unrecoverable/mod.rs | 8 -------- 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/src/sync/mailbox.rs b/src/sync/mailbox.rs index 394483c..0e406e5 100644 --- a/src/sync/mailbox.rs +++ b/src/sync/mailbox.rs @@ -3,7 +3,7 @@ use crate::{ interrupt::context_switch, schedule::{current, scheduler::Scheduler}, task::{Task, TaskState}, - time, unrecoverable, + time, }; use alloc::sync::Arc; use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -178,7 +178,12 @@ impl Mailbox { /// /// NOTE: *must not* call this method in ISR context. pub fn wait(&self) { - unrecoverable::die_if_in_isr(); + // The application logic must have gone terribly wrong if the task + // tries to block when the scheduler is suspended or if an ISR + // tries to block. In this case, panic the task or ISR. + if Scheduler::is_suspended() || current::is_in_isr_context() { + panic!(); + } let mut should_block = true; @@ -230,7 +235,12 @@ impl Mailbox { /// /// NOTE: *must not* call this method in ISR context. pub fn wait_until_timeout(&self, timeout_ms: u32) -> bool { - unrecoverable::die_if_in_isr(); + // The application logic must have gone terribly wrong if the task + // tries to block when the scheduler is suspended or if an ISR + // tries to block. In this case, panic the task or ISR. + if Scheduler::is_suspended() || current::is_in_isr_context() { + panic!(); + } let mut should_block = true; diff --git a/src/sync/wait_queue.rs b/src/sync/wait_queue.rs index 40c10a4..840d1b4 100644 --- a/src/sync/wait_queue.rs +++ b/src/sync/wait_queue.rs @@ -5,7 +5,6 @@ use crate::{ interrupt::context_switch, schedule::{current, scheduler::Scheduler}, task::{TaskListAdapter, TaskListInterfaces, TaskState}, - unrecoverable, }; use core::sync::atomic::{AtomicUsize, Ordering}; use intrusive_collections::LinkedList; @@ -97,8 +96,6 @@ impl WaitQueue { #[inline] #[allow(unused)] pub(super) fn wait(&self) { - unrecoverable::die_if_in_isr(); - add_cur_task_to_block_queue(self); // We have put the current task to the wait queue. @@ -108,6 +105,13 @@ impl WaitQueue { // Outline the logic to reduce the stack frame size of `.wait()`. #[inline(never)] fn add_cur_task_to_block_queue(wq: &WaitQueue) { + // The application logic must have gone terribly wrong if the task + // tries to block when the scheduler is suspended or if an ISR + // tries to block. In this case, panic the task or ISR. + if Scheduler::is_suspended() || current::is_in_isr_context() { + panic!(); + } + // Should always grant full access to a task. wq.inner.with_suspended_scheduler(|queue, sched_guard| { queue.must_with_full_access(|full_access| { @@ -140,8 +144,6 @@ impl WaitQueue { where F: FnMut() -> Option, { - unrecoverable::die_if_in_isr(); - // Keep blocking until the predicate is satisfied. loop { let res = add_cur_task_to_block_queue_with_condition(self, &mut condition); @@ -165,6 +167,13 @@ impl WaitQueue { where F: FnMut() -> Option, { + // The application logic must have gone terribly wrong if the task + // tries to block when the scheduler is suspended or if an ISR + // tries to block. In this case, panic the task or ISR. + if Scheduler::is_suspended() || current::is_in_isr_context() { + panic!(); + } + // Should always grant full access to a task. wq.inner.with_suspended_scheduler(|queue, sched_guard| { queue.must_with_full_access(|full_access| { @@ -216,8 +225,6 @@ impl WaitQueue { G: UnlockableGuard<'a, LockType = L>, L: Lockable = G> + 'a, { - unrecoverable::die_if_in_isr(); - // Keep blocking until the predicate is satisfied. loop { let res = add_cur_task_to_block_queue_and_unlock(self, guard, &mut condition); @@ -248,6 +255,13 @@ impl WaitQueue { G: UnlockableGuard<'a, LockType = L>, L: Lockable = G> + 'a, { + // The application logic must have gone terribly wrong if the task + // tries to block when the scheduler is suspended or if an ISR + // tries to block. In this case, panic the task or ISR. + if Scheduler::is_suspended() || current::is_in_isr_context() { + panic!(); + } + // Should always grant full access to a task. wq.inner.with_suspended_scheduler(|queue, sched_guard| { queue.must_with_full_access(|full_access| { diff --git a/src/time/mod.rs b/src/time/mod.rs index 1c9aa30..79c1270 100644 --- a/src/time/mod.rs +++ b/src/time/mod.rs @@ -156,6 +156,13 @@ fn sleep_ms_unchecked(ms: u32) { // Outline the logic to reduce the stack frame size of `sleep_ms`. #[inline(never)] fn add_cur_task_to_sleep_queue(wake_at_tick: u32) { + // The application logic must have gone terribly wrong if the task + // tries to block when the scheduler is suspended or if an ISR + // tries to block. In this case, panic the task or ISR. + if Scheduler::is_suspended() || current::is_in_isr_context() { + panic!(); + } + current::with_cur_task_arc(|cur_task| { cur_task.set_state(TaskState::Blocked); add_task_to_sleep_queue(cur_task, wake_at_tick); diff --git a/src/unrecoverable/mod.rs b/src/unrecoverable/mod.rs index a9a1e68..262cd31 100644 --- a/src/unrecoverable/mod.rs +++ b/src/unrecoverable/mod.rs @@ -1,5 +1,3 @@ -use crate::schedule::current; - pub trait Lethal { type T; fn unwrap_or_die(self) -> Self::T; @@ -36,9 +34,3 @@ where die() } } - -pub fn die_if_in_isr() { - if current::is_in_isr_context() { - die(); - } -}