kernel: Leave reaping to the reap thread
Some checks failed
Build and test / build (push) Has been cancelled

This seems to fix previous problems. Apparently reaping a thread somewhat corrupts/replaces the calling thread's address space.

I should've known there's a reason we do it in a separate kernel thread...
This commit is contained in:
apio 2024-12-07 13:05:38 +01:00
parent 853a6d7b38
commit 42afef5ccb
Signed by: apio
GPG Key ID: B8A7D06E42258954
7 changed files with 53 additions and 8 deletions

View File

@ -26,9 +26,11 @@ void reap_thread()
{ {
CPU::disable_interrupts(); CPU::disable_interrupts();
auto dying_threads = Scheduler::check_for_dying_threads(); auto dying_threads = Scheduler::check_for_dying_threads();
auto dead_processes = Scheduler::check_for_dead_processes();
CPU::enable_interrupts(); CPU::enable_interrupts();
dying_threads.consume([](Thread* thread) { Scheduler::reap_thread(thread); }); dying_threads.consume([](Thread* thread) { Scheduler::reap_thread(thread); });
dead_processes.consume([](Process* p) { Scheduler::reap_process(p); });
kernel_wait_for_event(); kernel_wait_for_event();
} }

View File

@ -113,6 +113,7 @@ Result<u64> sys_execve(Registers* regs, SyscallArgs args)
if (t != thread) t->quit(); if (t != thread) t->quit();
return true; return true;
}); });
Scheduler::signal_reap_thread();
current->thread_count = 1; current->thread_count = 1;
@ -145,6 +146,7 @@ Result<u64> sys_execve(Registers* regs, SyscallArgs args)
} }
current->cmdline = cmdline.chars(); current->cmdline = cmdline.chars();
thread->cmdline = cmdline.chars();
image->apply(thread); image->apply(thread);

View File

@ -25,8 +25,8 @@ Result<u64> sys_waitpid(Registers* regs, SyscallArgs args)
if (options & WNOHANG) return err(EAGAIN); if (options & WNOHANG) return err(EAGAIN);
wait_for_child: wait_for_child:
if (target->alive()) kernel_wait(pid); if (!target->dead()) kernel_wait(pid);
if (current->interrupted) if (current->interrupted && current->pending_signal() != SIGCHLD)
{ {
kdbgln("signal: waitpid interrupted by signal"); kdbgln("signal: waitpid interrupted by signal");
if (current->will_ignore_pending_signal()) if (current->will_ignore_pending_signal())
@ -37,7 +37,7 @@ Result<u64> sys_waitpid(Registers* regs, SyscallArgs args)
return err(EINTR); return err(EINTR);
} }
check(!target->alive()); check(target->dead());
} }
else if (pid == -1) else if (pid == -1)
{ {
@ -50,7 +50,7 @@ Result<u64> sys_waitpid(Registers* regs, SyscallArgs args)
wait_for_any_child: wait_for_any_child:
kernel_wait(pid); kernel_wait(pid);
if (current->interrupted) if (current->interrupted && current->pending_signal() != SIGCHLD)
{ {
kdbgln("signal: waitpid interrupted by signal"); kdbgln("signal: waitpid interrupted by signal");
if (current->will_ignore_pending_signal()) if (current->will_ignore_pending_signal())
@ -64,7 +64,7 @@ Result<u64> sys_waitpid(Registers* regs, SyscallArgs args)
check(current->child_being_waited_for != -1); check(current->child_being_waited_for != -1);
target = TRY(Result<Process*>::from_option(Scheduler::find_by_pid(current->child_being_waited_for), ESRCH)); target = TRY(Result<Process*>::from_option(Scheduler::find_by_pid(current->child_being_waited_for), ESRCH));
check(!target->alive()); check(!target->dead());
} }
else else
target = child.value(); target = child.value();
@ -80,7 +80,8 @@ Result<u64> sys_waitpid(Registers* regs, SyscallArgs args)
current->process->user_ticks_children += target->user_ticks_self + target->user_ticks_children; current->process->user_ticks_children += target->user_ticks_self + target->user_ticks_children;
current->process->kernel_ticks_children += target->kernel_ticks_self + target->kernel_ticks_children; current->process->kernel_ticks_children += target->kernel_ticks_self + target->kernel_ticks_children;
Scheduler::reap_process(target); target->thread_count = PROCESS_SHOULD_REAP;
Scheduler::signal_reap_thread();
if (status_ptr) if (status_ptr)
if (!MemoryManager::copy_to_user_typed(status_ptr, &status)) return err(EFAULT); if (!MemoryManager::copy_to_user_typed(status_ptr, &status)) return err(EFAULT);

View File

@ -117,6 +117,7 @@ namespace Scheduler
process->thread_count = 1; process->thread_count = 1;
process->virtual_clock.set_resolution(1'000'000); process->virtual_clock.set_resolution(1'000'000);
process->profiling_clock.set_resolution(1'000'000); process->profiling_clock.set_resolution(1'000'000);
process->cmdline = name;
process->is_kernel = true; process->is_kernel = true;
g_threads.append(thread); g_threads.append(thread);
@ -172,6 +173,7 @@ namespace Scheduler
process->id = 1; process->id = 1;
process->pgid = 1; process->pgid = 1;
process->thread_count = 1; process->thread_count = 1;
process->cmdline = name;
Vector<String> args; Vector<String> args;
auto name_string = TRY(String::from_cstring(name)); auto name_string = TRY(String::from_cstring(name));
@ -228,6 +230,8 @@ namespace Scheduler
void reap_process(Process* process) void reap_process(Process* process)
{ {
CPU::disable_interrupts();
// FIXME: Shouldn't all this be done when the timers' destructors are called? // FIXME: Shouldn't all this be done when the timers' destructors are called?
process->real_timer.disarm(); process->real_timer.disarm();
process->virtual_timer.disarm(); process->virtual_timer.disarm();
@ -238,6 +242,8 @@ namespace Scheduler
} }
delete process; delete process;
CPU::enable_interrupts();
} }
void reap_thread(Thread* thread) void reap_thread(Thread* thread)
@ -378,6 +384,21 @@ namespace Scheduler
return result; return result;
} }
LinkedList<Process> check_for_dead_processes()
{
LinkedList<Process> result;
g_processes.delayed_for_each([&](Process* p) {
if (p->thread_count == PROCESS_SHOULD_REAP)
{
g_processes.remove(p);
result.append(p);
}
});
return result;
}
Option<Process*> find_by_pid(pid_t pid) Option<Process*> find_by_pid(pid_t pid)
{ {
for (auto* const process : g_processes) for (auto* const process : g_processes)
@ -405,7 +426,7 @@ namespace Scheduler
Option<Process*> result; Option<Process*> result;
for_each_child(process, [&](Process* child) { for_each_child(process, [&](Process* child) {
if (!result.has_value() && !child->alive()) if (!result.has_value() && child->dead())
{ {
result = child; result = child;
return false; return false;
@ -469,6 +490,7 @@ void kernel_wait_for_event()
[[noreturn]] void kernel_exit() [[noreturn]] void kernel_exit()
{ {
g_current->state = ThreadState::Dying; g_current->state = ThreadState::Dying;
g_current->process->thread_count = PROCESS_SHOULD_REAP;
Scheduler::signal_reap_thread(); Scheduler::signal_reap_thread();
kernel_yield(); kernel_yield();
unreachable(); unreachable();

View File

@ -35,6 +35,7 @@ namespace Scheduler
void invoke(Registers* regs); void invoke(Registers* regs);
LinkedList<Thread> check_for_dying_threads(); LinkedList<Thread> check_for_dying_threads();
LinkedList<Process> check_for_dead_processes();
Option<Process*> find_by_pid(pid_t pid); Option<Process*> find_by_pid(pid_t pid);

View File

@ -137,6 +137,7 @@ Result<SharedPtr<VFS::Inode>> Process::resolve_atfile(int dirfd, const String& p
thread->quit(); thread->quit();
return true; return true;
}); });
Scheduler::signal_reap_thread();
thread_count = 0; thread_count = 0;
@ -276,6 +277,15 @@ void Thread::process_pending_signals(Registers* current_regs)
} }
} }
int Thread::pending_signal()
{
for (int i = 0; i < NSIG; i++)
{
if (pending_signals.get(i)) { return i + 1; }
}
return 0;
}
bool Thread::will_ignore_pending_signal() bool Thread::will_ignore_pending_signal()
{ {
for (int i = 0; i < NSIG; i++) for (int i = 0; i < NSIG; i++)

View File

@ -20,6 +20,7 @@
#endif #endif
constexpr int MAX_POSIX_TIMERS = 64; constexpr int MAX_POSIX_TIMERS = 64;
constexpr i64 PROCESS_SHOULD_REAP = -1;
class Timer; class Timer;
@ -49,7 +50,7 @@ struct Credentials
struct Process : public LinkedListNode<Process> struct Process : public LinkedListNode<Process>
{ {
Atomic<usize> thread_count; Atomic<i64> thread_count;
pid_t id; pid_t id;
Atomic<pid_t> pgid { 0 }; Atomic<pid_t> pgid { 0 };
@ -122,6 +123,11 @@ struct Process : public LinkedListNode<Process>
return thread_count > 0; return thread_count > 0;
} }
bool dead()
{
return thread_count == 0;
}
static Process* current(); static Process* current();
[[noreturn]] void exit(int status); [[noreturn]] void exit(int status);
@ -202,6 +208,7 @@ struct Thread : public LinkedListNode<Thread>
void process_pending_signals(Registers* current_regs); void process_pending_signals(Registers* current_regs);
int pending_signal();
bool will_ignore_pending_signal(); bool will_ignore_pending_signal();
bool deliver_signal(int signo, Registers* current_regs); bool deliver_signal(int signo, Registers* current_regs);