Skip to content

Commit f6239c5

Browse files
andyrosskartben
authored andcommitted
kernel/sched: Panic after aborting essential thread, not before
The essential thread check and panic happens at the top of k_thread_abort(). This is arguably a performance bug: the system is going to blow up anyway no matter where we put the test, we shouldn't add instructions to the path taken by systems that DON'T blow up. But really it's more of a testability/robustness glitch: if you have a fatal error handler that wants to catch this panic (say, a test using ztest_set_fault_valid()), then the current code will panic and early-exit BEFORE THE THREAD IS DEAD. And so it won't actually die, and will continue on causing mayhem when presumably the handler code expected it to have been aborted. It's sort of an unanswerable question as to what the "right" behavior is here (the system is, after all, supposed to have panicked!). But this seems preferable for definable practical reasons. Kill the thread, then panic. Unless it's _current, in which case panic as late as possible for maximum coverage of the abort path. Fixes: #84460 Signed-off-by: Andy Ross <[email protected]>
1 parent 446ad09 commit f6239c5

File tree

1 file changed

+9
-7
lines changed

1 file changed

+9
-7
lines changed

kernel/sched.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,9 @@ static ALWAYS_INLINE void z_thread_halt(struct k_thread *thread, k_spinlock_key_
433433
} else {
434434
halt_thread(thread, terminate ? _THREAD_DEAD : _THREAD_SUSPENDED);
435435
if ((thread == _current) && !arch_is_in_isr()) {
436+
if (z_is_thread_essential(thread)) {
437+
k_panic();
438+
}
436439
z_swap(&_sched_spinlock, key);
437440
__ASSERT(!terminate, "aborted _current back from dead");
438441
} else {
@@ -1316,21 +1319,20 @@ static ALWAYS_INLINE void halt_thread(struct k_thread *thread, uint8_t new_state
13161319

13171320
void z_thread_abort(struct k_thread *thread)
13181321
{
1322+
bool essential = z_is_thread_essential(thread);
13191323
k_spinlock_key_t key = k_spin_lock(&_sched_spinlock);
13201324

1321-
if (z_is_thread_essential(thread)) {
1322-
k_spin_unlock(&_sched_spinlock, key);
1323-
__ASSERT(false, "aborting essential thread %p", thread);
1324-
k_panic();
1325-
return;
1326-
}
1327-
13281325
if ((thread->base.thread_state & _THREAD_DEAD) != 0U) {
13291326
k_spin_unlock(&_sched_spinlock, key);
13301327
return;
13311328
}
13321329

13331330
z_thread_halt(thread, key, true);
1331+
1332+
if (essential) {
1333+
__ASSERT(!essential, "aborted essential thread %p", thread);
1334+
k_panic();
1335+
}
13341336
}
13351337

13361338
#if !defined(CONFIG_ARCH_HAS_THREAD_ABORT)

0 commit comments

Comments
 (0)