From 2d3810874d023e9f0cf41cfbdc0873c407b70224 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Sun, 26 May 2024 20:08:35 +0300 Subject: [PATCH] Kernel: Fix thread signal handling Threads will now only handle signals once they are not holding any mutexes. This removes some dead locks. --- kernel/include/kernel/Lock/Mutex.h | 14 ++++++++++++++ kernel/include/kernel/Thread.h | 10 +++++++--- kernel/kernel/Thread.cpp | 14 ++++++-------- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/kernel/include/kernel/Lock/Mutex.h b/kernel/include/kernel/Lock/Mutex.h index f71187e2..431bafbb 100644 --- a/kernel/include/kernel/Lock/Mutex.h +++ b/kernel/include/kernel/Lock/Mutex.h @@ -27,6 +27,8 @@ namespace Kernel while (!m_locker.compare_exchange(-1, tid)) Scheduler::get().yield(); ASSERT(m_lock_depth == 0); + if (Scheduler::current_tid()) + Thread::current().add_mutex(); } m_lock_depth++; } @@ -41,6 +43,8 @@ namespace Kernel if (!m_locker.compare_exchange(-1, tid)) return false; ASSERT(m_lock_depth == 0); + if (Scheduler::current_tid()) + Thread::current().add_mutex(); } m_lock_depth++; return true; @@ -51,7 +55,11 @@ namespace Kernel ASSERT(m_locker == Scheduler::current_tid()); ASSERT(m_lock_depth > 0); if (--m_lock_depth == 0) + { m_locker = -1; + if (Scheduler::current_tid()) + Thread::current().remove_mutex(); + } } pid_t locker() const { return m_locker; } @@ -84,6 +92,8 @@ namespace Kernel while (!(has_priority || m_queue_length == 0) || !m_locker.compare_exchange(-1, tid)) Scheduler::get().yield(); ASSERT(m_lock_depth == 0); + if (Scheduler::current_tid()) + Thread::current().add_mutex(); } m_lock_depth++; } @@ -101,6 +111,8 @@ namespace Kernel if (has_priority) m_queue_length++; ASSERT(m_lock_depth == 0); + if (Scheduler::current_tid()) + Thread::current().add_mutex(); } m_lock_depth++; return true; @@ -117,6 +129,8 @@ namespace Kernel if (has_priority) m_queue_length--; m_locker = -1; + if (Scheduler::current_tid()) + Thread::current().remove_mutex(); } } diff --git a/kernel/include/kernel/Thread.h b/kernel/include/kernel/Thread.h index 9da92222..ff11e345 100644 --- a/kernel/include/kernel/Thread.h +++ b/kernel/include/kernel/Thread.h @@ -38,9 +38,8 @@ namespace Kernel void setup_exec(); void setup_process_cleanup(); - // Adds pending signals to thread if possible and - // returns true, if thread is going to trigger signal - bool is_interrupted_by_signal(); + // Returns true, if thread is going to trigger signal + bool is_interrupted_by_signal() const; // Returns true if pending signal can be added to thread bool can_add_signal_to_execute() const; @@ -86,6 +85,9 @@ namespace Kernel static Thread* sse_thread(); #endif + void add_mutex() { m_mutex_count++; } + void remove_mutex() { m_mutex_count--; } + private: Thread(pid_t tid, Process*); @@ -111,6 +113,8 @@ namespace Kernel SpinLock m_signal_lock; static_assert(_SIGMAX < 64); + BAN::Atomic m_mutex_count { 0 }; + #if __enable_sse alignas(16) uint8_t m_sse_storage[512] {}; #endif diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp index d98a13b6..7ecef600 100644 --- a/kernel/kernel/Thread.cpp +++ b/kernel/kernel/Thread.cpp @@ -272,14 +272,7 @@ namespace Kernel memset(&m_interrupt_registers, 0, sizeof(InterruptRegisters)); } - bool Thread::is_interrupted_by_signal() - { - while (can_add_signal_to_execute()) - handle_signal(); - return will_execute_signal(); - } - - bool Thread::can_add_signal_to_execute() const + bool Thread::is_interrupted_by_signal() const { if (!is_userspace() || m_state != State::Executing) return false; @@ -290,6 +283,11 @@ namespace Kernel return full_pending_mask & ~m_signal_block_mask; } + bool Thread::can_add_signal_to_execute() const + { + return is_interrupted_by_signal() && m_mutex_count == 0; + } + bool Thread::will_execute_signal() const { if (!is_userspace() || m_state != State::Executing)