Kernel: Keep signal state in sync

In c3d2316 we added the atomic variable
m_have_any_unmasked_pending_signals tracking the state of pending signals.
Add helper functions that automatically update this variable as needed.

Kernel/Syscalls/execve.cpp

@@ -235,8 +235,7 @@ int Process::do_exec(NonnullRefPtr<FileDescription> main_program_description, Ve
  }
 
  current_thread->set_default_signal_dispositions();
- current_thread->m_signal_mask = 0;
- current_thread->m_pending_signals = 0;
+ current_thread->clear_signals();
 
  m_futex_queues.clear();
 

Kernel/Syscalls/select.cpp

@@ -24,7 +24,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include <AK/ScopedValueRollback.h>
+#include <AK/ScopeGuard.h>
 #include <AK/Time.h>
 #include <Kernel/FileSystem/FileDescription.h>
 #include <Kernel/Process.h>
@@ -73,9 +73,14 @@ int Process::sys$select(const Syscall::SC_select_params* params)
  }
 
  auto current_thread = Thread::current();
- ScopedValueRollback scoped_sigmask(current_thread->m_signal_mask);
+
+ u32 previous_signal_mask = 0;
  if (sigmask)
- current_thread->m_signal_mask = *sigmask;
+ previous_signal_mask = current_thread->update_signal_mask(*sigmask);
+ ScopeGuard rollback_signal_mask([&]() {
+ if (sigmask)
+ current_thread->update_signal_mask(previous_signal_mask);
+ });
 
  Thread::SelectBlocker::FDVector rfds;
  Thread::SelectBlocker::FDVector wfds;
@@ -187,9 +192,14 @@ int Process::sys$poll(Userspace<const Syscall::SC_poll_params*> user_params)
  }
 
  auto current_thread = Thread::current();
- ScopedValueRollback scoped_sigmask(current_thread->m_signal_mask);
+
+ u32 previous_signal_mask = 0;
  if (params.sigmask)
- current_thread->m_signal_mask = sigmask;
+ previous_signal_mask = current_thread->update_signal_mask(params.sigmask);
+ ScopeGuard rollback_signal_mask([&]() {
+ if (params.sigmask)
+ current_thread->update_signal_mask(previous_signal_mask);
+ });
 
 #if defined(DEBUG_IO) || defined(DEBUG_POLL_SELECT)
  dbg() << "polling on (read:" << rfds.size() << ", write:" << wfds.size() << "), timeout=" << timeout.tv_sec << "s" << timeout.tv_nsec << "ns";

Kernel/Syscalls/sigaction.cpp

@@ -32,29 +32,32 @@ int Process::sys$sigprocmask(int how, Userspace<const sigset_t*> set, Userspace<
 {
  REQUIRE_PROMISE(sigaction);
  auto current_thread = Thread::current();
- if (old_set) {
- if (!validate_write_typed(old_set))
- return -EFAULT;
- copy_to_user(old_set, &current_thread->m_signal_mask);
- }
+ u32 previous_signal_mask;
  if (set) {
  if (!validate_read_typed(set))
  return -EFAULT;
  sigset_t set_value;
  copy_from_user(&set_value, set);
  switch (how) {
  case SIG_BLOCK:
- current_thread->m_signal_mask &= ~set_value;
+ previous_signal_mask = current_thread->signal_mask_block(set_value, true);
  break;
  case SIG_UNBLOCK:
- current_thread->m_signal_mask |= set_value;
+ previous_signal_mask = current_thread->signal_mask_block(set_value, false);
  break;
  case SIG_SETMASK:
- current_thread->m_signal_mask = set_value;
+ previous_signal_mask = current_thread->update_signal_mask(set_value);
  break;
  default:
  return -EINVAL;
  }
+ } else {
+ previous_signal_mask = current_thread->signal_mask();
+ }
+ if (old_set) {
+ if (!validate_write_typed(old_set))
+ return -EFAULT;
+ copy_to_user(old_set, &previous_signal_mask);
  }
  return 0;
 }
@@ -64,7 +67,8 @@ int Process::sys$sigpending(Userspace<sigset_t*> set)
  REQUIRE_PROMISE(stdio);
  if (!validate_write_typed(set))
  return -EFAULT;
- copy_to_user(set, &Thread::current()->m_pending_signals);
+ auto pending_signals = Thread::current()->pending_signals();
+ copy_to_user(set, &pending_signals);
  return 0;
 }
 

Kernel/Thread.cpp

@@ -308,10 +308,22 @@ bool Thread::tick()
  return --m_ticks_left;
 }
 
+bool Thread::has_pending_signal(u8 signal) const
+{
+ ScopedSpinLock lock(g_scheduler_lock);
+ return m_pending_signals & (1 << (signal - 1));
+}
+
+u32 Thread::pending_signals() const
+{
+ ScopedSpinLock lock(g_scheduler_lock);
+ return m_pending_signals;
+}
+
 void Thread::send_signal(u8 signal, [[maybe_unused]] Process* sender)
 {
  ASSERT(signal < 32);
- InterruptDisabler disabler;
+ ScopedSpinLock lock(g_scheduler_lock);
 
  // FIXME: Figure out what to do for masked signals. Should we also ignore them here?
  if (should_ignore_signal(signal)) {
@@ -328,11 +340,45 @@ void Thread::send_signal(u8 signal, [[maybe_unused]] Process* sender)
  dbg() << "Signal: Kernel sent " << signal << " to " << process();
 #endif
 
- ScopedSpinLock lock(g_scheduler_lock);
  m_pending_signals |= 1 << (signal - 1);
  m_have_any_unmasked_pending_signals.store(m_pending_signals & ~m_signal_mask, AK::memory_order_release);
 }
 
+u32 Thread::update_signal_mask(u32 signal_mask)
+{
+ ScopedSpinLock lock(g_scheduler_lock);
+ auto previous_signal_mask = m_signal_mask;
+ m_signal_mask = signal_mask;
+ m_have_any_unmasked_pending_signals.store(m_pending_signals & ~m_signal_mask, AK::memory_order_release);
+ return previous_signal_mask;
+}
+
+u32 Thread::signal_mask() const
+{
+ ScopedSpinLock lock(g_scheduler_lock);
+ return m_signal_mask;
+}
+
+u32 Thread::signal_mask_block(sigset_t signal_set, bool block)
+{
+ ScopedSpinLock lock(g_scheduler_lock);
+ auto previous_signal_mask = m_signal_mask;
+ if (block)
+ m_signal_mask &= ~signal_set;
+ else
+ m_signal_mask |= signal_set;
+ m_have_any_unmasked_pending_signals.store(m_pending_signals & ~m_signal_mask, AK::memory_order_release);
+ return previous_signal_mask;
+}
+
+void Thread::clear_signals()
+{
+ ScopedSpinLock lock(g_scheduler_lock);
+ m_signal_mask = 0;
+ m_pending_signals = 0;
+ m_have_any_unmasked_pending_signals.store(false, AK::memory_order_release);
+}
+
 // Certain exceptions, such as SIGSEGV and SIGILL, put a
 // thread into a state where the signal handler must be
 // invoked immediately, otherwise it will continue to fault.

Kernel/Thread.h

@@ -411,6 +411,11 @@ class Thread {
  void send_signal(u8 signal, Process* sender);
  void consider_unblock(time_t now_sec, long now_usec);
 
+ u32 update_signal_mask(u32 signal_mask);
+ u32 signal_mask_block(sigset_t signal_set, bool block);
+ u32 signal_mask() const;
+ void clear_signals();
+
  void set_dump_backtrace_on_finalization() { m_dump_backtrace_on_finalization = true; }
 
  ShouldUnblockThread dispatch_one_pending_signal();
@@ -419,7 +424,8 @@ class Thread {
  void terminate_due_to_signal(u8 signal);
  bool should_ignore_signal(u8 signal) const;
  bool has_signal_handler(u8 signal) const;
- bool has_pending_signal(u8 signal) const { return m_pending_signals & (1 << (signal - 1)); }
+ bool has_pending_signal(u8 signal) const;
+ u32 pending_signals() const;
 
  FPUState& fpu_state() { return *m_fpu_state; }