From ceb24341cdc8088fdd67404d424b82d2cc006b1b Mon Sep 17 00:00:00 2001 From: Kai Law Date: Thu, 5 Mar 2026 11:35:24 +0800 Subject: [PATCH 1/2] Thread being retired should not be preempted Fix https://github.com/vivoblueos/kernel/actions/runs/22700766319/job/65817293534. ``` panicked at ../../kernel/kernel/src/scheduler/mod.rs:269:9: assertion `left == right` failed left: Err(4) right: Ok(()) Oops: assertion `left == right` failed ``` --- kernel/src/scheduler/mod.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/kernel/src/scheduler/mod.rs b/kernel/src/scheduler/mod.rs index 31f14587..77963b3c 100644 --- a/kernel/src/scheduler/mod.rs +++ b/kernel/src/scheduler/mod.rs @@ -231,18 +231,18 @@ fn switch_current_thread(next: ThreadNode, old_sp: usize) -> usize { #[cfg(thread_stats)] old.increment_cycles(cycles); if old.state() == thread::RETIRED { - let cleanup = old.lock().take_cleanup(); - if let Some(entry) = cleanup { + old.enable_preempt(); + GlobalQueueVisitor::remove(&mut old); + if ThreadNode::strong_count(&old) != 1 { + // TODO: Add warning log that there are still references to the old thread. + } + if let Some(entry) = old.lock().take_cleanup() { match entry { Entry::C(f) => f(), Entry::Closure(f) => f(), Entry::Posix(f, arg) => f(arg), } }; - GlobalQueueVisitor::remove(&mut old); - if ThreadNode::strong_count(&old) != 1 { - // TODO: Add warning log that there are still references to the old thread. - } } old.set_saved_sp(old_sp); next_saved_sp @@ -272,14 +272,16 @@ pub(crate) extern "C" fn relinquish_me_and_return_next_sp(old_sp: usize) -> usiz } pub fn retire_me() -> ! { + let retiring = current_thread_ref(); #[cfg(procfs)] { - let _ = crate::vfs::trace_thread_close(current_thread()); + let _ = crate::vfs::trace_thread_close(unsafe { Arc::clone_from(retiring) }); } let next = next_ready_thread().map_or_else(idle::current_idle_thread, |v| v); debug_assert_eq!(next.state(), thread::READY); let mut hooks = ContextSwitchHookHolder::new(next); - let ok = current_thread_ref().transfer_state(thread::RUNNING, thread::RETIRED); + retiring.disable_preempt(); + let ok = retiring.transfer_state(thread::RUNNING, thread::RETIRED); debug_assert_eq!(ok, Ok(())); arch::switch_context_with_hook(&mut hooks as *mut _); unreachable!("Retired thread should not reach here") From 85237406ebfd24b2861c2f15093c2ff79ff2f5a7 Mon Sep 17 00:00:00 2001 From: Kai Law Date: Thu, 5 Mar 2026 18:47:19 +0800 Subject: [PATCH 2/2] Address comment --- kernel/src/lib.rs | 1 + kernel/src/scheduler/mod.rs | 1 - kernel/src/thread/mod.rs | 5 +++++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/kernel/src/lib.rs b/kernel/src/lib.rs index de2877f0..a793aed7 100644 --- a/kernel/src/lib.rs +++ b/kernel/src/lib.rs @@ -174,6 +174,7 @@ mod tests { ) { unsafe { let t = TEST_THREADS[i].assume_init_ref(); + t.set_preempt_count(0); let mut w = t.lock(); let stack = &mut TEST_THREAD_STORAGES[i].stack; let Some(stack) = thread::Stack::from_raw(stack.rep.as_mut_ptr(), stack.rep.len()) diff --git a/kernel/src/scheduler/mod.rs b/kernel/src/scheduler/mod.rs index 77963b3c..ef233c8c 100644 --- a/kernel/src/scheduler/mod.rs +++ b/kernel/src/scheduler/mod.rs @@ -231,7 +231,6 @@ fn switch_current_thread(next: ThreadNode, old_sp: usize) -> usize { #[cfg(thread_stats)] old.increment_cycles(cycles); if old.state() == thread::RETIRED { - old.enable_preempt(); GlobalQueueVisitor::remove(&mut old); if ThreadNode::strong_count(&old) != 1 { // TODO: Add warning log that there are still references to the old thread. diff --git a/kernel/src/thread/mod.rs b/kernel/src/thread/mod.rs index 7b60185c..28c6b4d0 100644 --- a/kernel/src/thread/mod.rs +++ b/kernel/src/thread/mod.rs @@ -634,6 +634,11 @@ impl Thread { self.preempt_count.load(Ordering::Acquire) } + pub unsafe fn set_preempt_count(&self, val: Uint) -> &Self { + self.preempt_count.store(val, Ordering::Release); + self + } + #[cfg(thread_stats)] #[inline] pub fn increment_cycles(&self, cycles: u64) {