diff options
author | Ali Mohammad Pur <ali.mpfard@gmail.com> | 2022-02-24 22:25:49 +0330 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2022-03-04 20:07:05 +0100 |
commit | cf63447044f7f26bc955d6dbfceb10eedfbd3112 (patch) | |
tree | 9c244265fd0b8f39efddcb5c837e5cc315cfbef4 /Kernel | |
parent | 18b9d02edd427589eaa7fb7da1a7f6d6696cff4f (diff) | |
download | serenity-cf63447044f7f26bc955d6dbfceb10eedfbd3112.zip |
Kernel: Move signal handlers from being thread state to process state
POSIX requires that sigaction() and friends set a _process-wide_ signal
handler, so move signal handlers and flags inside Process.
This also fixes a "pid/tid confusion" FIXME, as we can now send the
signal to the process and let that decide which thread should get the
signal (which is the thread with tid==pid, but that's now the Process's
problem).
Note that each thread still retains its signal mask, as that is local to
each thread.
Diffstat (limited to 'Kernel')
-rw-r--r-- | Kernel/Process.cpp | 7 | ||||
-rw-r--r-- | Kernel/Process.h | 6 | ||||
-rw-r--r-- | Kernel/Syscalls/sigaction.cpp | 4 | ||||
-rw-r--r-- | Kernel/Thread.cpp | 13 | ||||
-rw-r--r-- | Kernel/Thread.h | 8 |
5 files changed, 19 insertions, 19 deletions
diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 2f637473fe..85d3e65802 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -612,10 +612,9 @@ void Process::finalize() m_state.store(State::Dead, AK::MemoryOrder::memory_order_release); { - // FIXME: PID/TID BUG - if (auto parent_thread = Thread::from_tid(ppid().value())) { - if (parent_thread->process().is_user_process() && (parent_thread->m_signal_action_data[SIGCHLD].flags & SA_NOCLDWAIT) != SA_NOCLDWAIT) - parent_thread->send_signal(SIGCHLD, this); + if (auto parent_process = Process::from_pid(ppid())) { + if (parent_process->is_user_process() && (parent_process->m_signal_action_data[SIGCHLD].flags & SA_NOCLDWAIT) != SA_NOCLDWAIT) + (void)parent_process->send_signal(SIGCHLD, this); } } diff --git a/Kernel/Process.h b/Kernel/Process.h index 7a89ac9891..97300c5d66 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -827,6 +827,12 @@ private: NonnullRefPtrVector<Thread> m_threads_for_coredump; mutable RefPtr<ProcessProcFSTraits> m_procfs_traits; + struct SignalActionData { + VirtualAddress handler_or_sigaction; + int flags { 0 }; + u32 mask { 0 }; + }; + Array<SignalActionData, NSIG> m_signal_action_data; static_assert(sizeof(ProtectedValues) < (PAGE_SIZE)); alignas(4096) ProtectedValues m_protected_values; diff --git a/Kernel/Syscalls/sigaction.cpp b/Kernel/Syscalls/sigaction.cpp index 79450ca974..80e14c44a8 100644 --- a/Kernel/Syscalls/sigaction.cpp +++ b/Kernel/Syscalls/sigaction.cpp @@ -58,15 +58,17 @@ ErrorOr<FlatPtr> Process::sys$sigaction(int signum, Userspace<const sigaction*> return EINVAL; InterruptDisabler disabler; // FIXME: This should use a narrower lock. Maybe a way to ignore signals temporarily? - auto& action = Thread::current()->m_signal_action_data[signum]; + auto& action = m_signal_action_data[signum]; if (user_old_act) { sigaction old_act {}; old_act.sa_flags = action.flags; old_act.sa_sigaction = reinterpret_cast<decltype(old_act.sa_sigaction)>(action.handler_or_sigaction.as_ptr()); + old_act.sa_mask = action.mask; TRY(copy_to_user(user_old_act, &old_act)); } if (user_act) { auto act = TRY(copy_typed_from_user(user_act)); + action.mask = act.sa_mask; action.flags = act.sa_flags; action.handler_or_sigaction = VirtualAddress { reinterpret_cast<void*>(act.sa_sigaction) }; } diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index ca7038f50d..7286b87c95 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -790,7 +790,7 @@ void Thread::reset_signals_for_exec() // The signal mask is preserved across execve(2). // The pending signal set is preserved across an execve(2). m_have_any_unmasked_pending_signals.store(false, AK::memory_order_release); - m_signal_action_data.fill({}); + m_signal_action_masks.fill({}); // A successful call to execve(2) removes any existing alternate signal stack m_alternative_signal_stack = 0; m_alternative_signal_stack_size = 0; @@ -902,7 +902,7 @@ static DefaultSignalAction default_signal_action(u8 signal) bool Thread::should_ignore_signal(u8 signal) const { VERIFY(signal < 32); - auto const& action = m_signal_action_data[signal]; + auto const& action = m_process->m_signal_action_data[signal]; if (action.handler_or_sigaction.is_null()) return default_signal_action(signal) == DefaultSignalAction::Ignore; return ((sighandler_t)action.handler_or_sigaction.get() == SIG_IGN); @@ -911,7 +911,7 @@ bool Thread::should_ignore_signal(u8 signal) const bool Thread::has_signal_handler(u8 signal) const { VERIFY(signal < 32); - auto const& action = m_signal_action_data[signal]; + auto const& action = m_process->m_signal_action_data[signal]; return !action.handler_or_sigaction.is_null(); } @@ -975,7 +975,7 @@ DispatchSignalResult Thread::dispatch_signal(u8 signal) return DispatchSignalResult::Deferred; } - auto& action = m_signal_action_data[signal]; + auto& action = m_process->m_signal_action_data[signal]; // FIXME: Implement SA_SIGINFO signal handlers. VERIFY(!(action.flags & SA_SIGINFO)); @@ -1045,7 +1045,7 @@ DispatchSignalResult Thread::dispatch_signal(u8 signal) ScopedAddressSpaceSwitcher switcher(m_process); u32 old_signal_mask = m_signal_mask; - u32 new_signal_mask = action.mask; + u32 new_signal_mask = m_signal_action_masks[signal].value_or(action.mask); if ((action.flags & SA_NODEFER) == SA_NODEFER) new_signal_mask &= ~(1 << (signal - 1)); else @@ -1182,8 +1182,7 @@ RegisterState& Thread::get_register_dump_from_stack() ErrorOr<NonnullRefPtr<Thread>> Thread::try_clone(Process& process) { auto clone = TRY(Thread::try_create(process)); - auto signal_action_data_span = m_signal_action_data.span(); - signal_action_data_span.copy_to(clone->m_signal_action_data.span()); + m_signal_action_masks.span().copy_to(clone->m_signal_action_masks); clone->m_signal_mask = m_signal_mask; clone->m_fpu_state = m_fpu_state; clone->m_thread_specific_data = m_thread_specific_data; diff --git a/Kernel/Thread.h b/Kernel/Thread.h index 11d3363848..a0b5324300 100644 --- a/Kernel/Thread.h +++ b/Kernel/Thread.h @@ -46,12 +46,6 @@ enum class DispatchSignalResult { Continue }; -struct SignalActionData { - VirtualAddress handler_or_sigaction; - u32 mask { 0 }; - int flags { 0 }; -}; - struct ThreadSpecificData { ThreadSpecificData* self; }; @@ -1225,7 +1219,7 @@ private: NonnullOwnPtr<Memory::Region> m_kernel_stack_region; VirtualAddress m_thread_specific_data; Optional<Memory::VirtualRange> m_thread_specific_range; - Array<SignalActionData, NSIG> m_signal_action_data; + Array<Optional<u32>, NSIG> m_signal_action_masks; Blocker* m_blocker { nullptr }; Kernel::Mutex* m_blocking_mutex { nullptr }; u32 m_lock_requested_count { 0 }; |