diff --git a/Kernel/Net/NetworkTask.cpp b/Kernel/Net/NetworkTask.cpp index ff2816ea12e..913ad877c2f 100644 --- a/Kernel/Net/NetworkTask.cpp +++ b/Kernel/Net/NetworkTask.cpp @@ -61,7 +61,7 @@ static void handle_tcp(const IPv4Packet&, const timeval& packet_timestamp); void NetworkTask::spawn() { - Thread* thread = nullptr; + RefPtr thread; Process::create_kernel_process(thread, "NetworkTask", NetworkTask_main); } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index f9d6fe4bb00..2efa041f9c9 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -265,7 +265,7 @@ void Process::kill_all_threads() }); } -RefPtr Process::create_user_process(Thread*& first_thread, const String& path, uid_t uid, gid_t gid, ProcessID parent_pid, int& error, Vector&& arguments, Vector&& environment, TTY* tty) +RefPtr Process::create_user_process(RefPtr& first_thread, const String& path, uid_t uid, gid_t gid, ProcessID parent_pid, int& error, Vector&& arguments, Vector&& environment, TTY* tty) { auto parts = path.split('/'); if (arguments.is_empty()) { @@ -298,7 +298,7 @@ RefPtr Process::create_user_process(Thread*& first_thread, const String error = process->exec(path, move(arguments), move(environment)); if (error != 0) { dbg() << "Failed to exec " << path << ": " << error; - delete first_thread; + first_thread = nullptr; return {}; } @@ -311,7 +311,7 @@ RefPtr Process::create_user_process(Thread*& first_thread, const String return process; } -NonnullRefPtr Process::create_kernel_process(Thread*& first_thread, String&& name, void (*e)(), u32 affinity) +NonnullRefPtr Process::create_kernel_process(RefPtr& first_thread, String&& name, void (*e)(), u32 affinity) { auto process = adopt(*new Process(first_thread, move(name), (uid_t)0, (gid_t)0, ProcessID(0), true)); first_thread->tss().eip = (FlatPtr)e; @@ -327,7 +327,7 @@ NonnullRefPtr Process::create_kernel_process(Thread*& first_thread, Str return process; } -Process::Process(Thread*& first_thread, const String& name, uid_t uid, gid_t gid, ProcessID ppid, bool is_kernel_process, RefPtr cwd, RefPtr executable, TTY* tty, Process* fork_parent) +Process::Process(RefPtr& first_thread, const String& name, uid_t uid, gid_t gid, ProcessID ppid, bool is_kernel_process, RefPtr cwd, RefPtr executable, TTY* tty, Process* fork_parent) : m_name(move(name)) , m_pid(allocate_pid()) , m_euid(uid) @@ -356,14 +356,15 @@ Process::Process(Thread*& first_thread, const String& name, uid_t uid, gid_t gid first_thread = Thread::current()->clone(*this); } else { // NOTE: This non-forked code path is only taken when the kernel creates a process "manually" (at boot.) - first_thread = new Thread(*this); + first_thread = adopt(*new Thread(*this)); first_thread->detach(); } } Process::~Process() { - ASSERT(thread_count() == 0); + ASSERT(!m_next && !m_prev); // should have been reaped + ASSERT(thread_count() == 0); // all threads should have been finalized } void Process::dump_regions() @@ -613,7 +614,7 @@ void Process::finalize() { InterruptDisabler disabler; // FIXME: PID/TID BUG - if (auto* parent_thread = Thread::from_tid(m_ppid.value())) { + if (auto parent_thread = Thread::from_tid(m_ppid.value())) { if (parent_thread->m_signal_action_data[SIGCHLD].flags & SA_NOCLDWAIT) { // NOTE: If the parent doesn't care about this process, let it go. m_ppid = 0; @@ -761,13 +762,13 @@ KResult Process::send_signal(u8 signal, Process* sender) return KResult(-ESRCH); } -Thread* Process::create_kernel_thread(void (*entry)(), u32 priority, const String& name, u32 affinity, bool joinable) +RefPtr Process::create_kernel_thread(void (*entry)(), u32 priority, const String& name, u32 affinity, bool joinable) { ASSERT((priority >= THREAD_PRIORITY_MIN) && (priority <= THREAD_PRIORITY_MAX)); // FIXME: Do something with guard pages? - auto* thread = new Thread(*this); + auto thread = adopt(*new Thread(*this)); thread->set_name(name); thread->set_affinity(affinity); diff --git a/Kernel/Process.h b/Kernel/Process.h index d84af27f699..743d654e8bb 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -125,14 +125,14 @@ public: return current_thread ? ¤t_thread->process() : nullptr; } - static NonnullRefPtr create_kernel_process(Thread*& first_thread, String&& name, void (*entry)(), u32 affinity = THREAD_AFFINITY_DEFAULT); - static RefPtr create_user_process(Thread*& first_thread, const String& path, uid_t, gid_t, ProcessID ppid, int& error, Vector&& arguments = Vector(), Vector&& environment = Vector(), TTY* = nullptr); + static NonnullRefPtr create_kernel_process(RefPtr& first_thread, String&& name, void (*entry)(), u32 affinity = THREAD_AFFINITY_DEFAULT); + static RefPtr create_user_process(RefPtr& first_thread, const String& path, uid_t, gid_t, ProcessID ppid, int& error, Vector&& arguments = Vector(), Vector&& environment = Vector(), TTY* = nullptr); ~Process(); static Vector all_pids(); static AK::NonnullRefPtrVector all_processes(); - Thread* create_kernel_thread(void (*entry)(), u32 priority, const String& name, u32 affinity = THREAD_AFFINITY_DEFAULT, bool joinable = true); + RefPtr create_kernel_thread(void (*entry)(), u32 priority, const String& name, u32 affinity = THREAD_AFFINITY_DEFAULT, bool joinable = true); bool is_profiling() const { return m_profiling; } void set_profiling(bool profiling) { m_profiling = profiling; } @@ -466,7 +466,7 @@ private: friend class Scheduler; friend class Region; - Process(Thread*& first_thread, const String& name, uid_t, gid_t, ProcessID ppid, bool is_kernel_process, RefPtr cwd = nullptr, RefPtr executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr); + Process(RefPtr& first_thread, const String& name, uid_t, gid_t, ProcessID ppid, bool is_kernel_process, RefPtr cwd = nullptr, RefPtr executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr); static ProcessID allocate_pid(); Range allocate_range(VirtualAddress, size_t, size_t alignment = PAGE_SIZE); diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index 6a4c3236fba..2a48537b598 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -50,7 +50,7 @@ class SchedulerPerProcessorData { public: SchedulerPerProcessorData() = default; - Thread* m_pending_beneficiary { nullptr }; + WeakPtr m_pending_beneficiary; const char* m_pending_donate_reason { nullptr }; bool m_in_scheduler { true }; }; @@ -599,7 +599,7 @@ bool Scheduler::donate_to_and_switch(Thread* beneficiary, const char* reason) return Scheduler::context_switch(beneficiary); } -bool Scheduler::donate_to(Thread* beneficiary, const char* reason) +bool Scheduler::donate_to(RefPtr& beneficiary, const char* reason) { ASSERT(beneficiary); @@ -625,7 +625,7 @@ bool Scheduler::donate_to(Thread* beneficiary, const char* reason) ASSERT(!proc.in_irq()); if (proc.in_critical() > 1) { - scheduler_data.m_pending_beneficiary = beneficiary; // Save the beneficiary + scheduler_data.m_pending_beneficiary = beneficiary->make_weak_ptr(); // Save the beneficiary scheduler_data.m_pending_donate_reason = reason; proc.invoke_scheduler_async(); return false; @@ -740,7 +740,7 @@ void Scheduler::initialize() { ASSERT(&Processor::current() != nullptr); // sanity check - Thread* idle_thread = nullptr; + RefPtr idle_thread; g_scheduler_data = new SchedulerData; g_finalizer_wait_queue = new WaitQueue; diff --git a/Kernel/Scheduler.h b/Kernel/Scheduler.h index e4b2184ce53..cfe27171915 100644 --- a/Kernel/Scheduler.h +++ b/Kernel/Scheduler.h @@ -60,7 +60,7 @@ public: static timeval time_since_boot(); static bool yield(); static bool donate_to_and_switch(Thread*, const char* reason); - static bool donate_to(Thread*, const char* reason); + static bool donate_to(RefPtr&, const char* reason); static bool context_switch(Thread*); static void enter_current(Thread& prev_thread); static void leave_on_first_switch(u32 flags); diff --git a/Kernel/Syscalls/fork.cpp b/Kernel/Syscalls/fork.cpp index d6ca53daf43..c4883911798 100644 --- a/Kernel/Syscalls/fork.cpp +++ b/Kernel/Syscalls/fork.cpp @@ -36,8 +36,10 @@ namespace Kernel { pid_t Process::sys$fork(RegisterState& regs) { REQUIRE_PROMISE(proc); - Thread* child_first_thread = nullptr; - auto* child = new Process(child_first_thread, m_name, m_uid, m_gid, m_pid, m_is_kernel_process, m_cwd, m_executable, m_tty, this); + RefPtr child_first_thread; + auto child = adopt(*new Process(child_first_thread, m_name, m_uid, m_gid, m_pid, m_is_kernel_process, m_cwd, m_executable, m_tty, this)); + if (!child_first_thread) + return -ENOMEM; child->m_root_directory = m_root_directory; child->m_root_directory_relative_to_global_root = m_root_directory_relative_to_global_root; child->m_promises = m_promises; @@ -92,6 +94,7 @@ pid_t Process::sys$fork(RegisterState& regs) { ScopedSpinLock lock(g_processes_lock); g_processes->prepend(child); + child->ref(); // This reference will be dropped by Process::reap } child_first_thread->set_affinity(Thread::current()->affinity()); diff --git a/Kernel/Syscalls/sched.cpp b/Kernel/Syscalls/sched.cpp index 91ca08884eb..8882a0c627d 100644 --- a/Kernel/Syscalls/sched.cpp +++ b/Kernel/Syscalls/sched.cpp @@ -40,8 +40,12 @@ int Process::sys$donate(pid_t tid) REQUIRE_PROMISE(stdio); if (tid < 0) return -EINVAL; - InterruptDisabler disabler; - auto* thread = Thread::from_tid(tid); + + // We don't strictly need to grab the scheduler lock here, but it + // will close a race where we can find the thread but it disappears + // before we call Scheduler::donate_to. + ScopedSpinLock lock(g_scheduler_lock); + auto thread = Thread::from_tid(tid); if (!thread || thread->pid() != pid()) return -ESRCH; Scheduler::donate_to(thread, "sys$donate"); @@ -55,8 +59,11 @@ int Process::sys$sched_setparam(int pid, Userspace us if (!copy_from_user(&desired_param, user_param)) return -EFAULT; - InterruptDisabler disabler; + if (desired_param.sched_priority < THREAD_PRIORITY_MIN || desired_param.sched_priority > THREAD_PRIORITY_MAX) + return -EINVAL; + auto* peer = Thread::current(); + ScopedSpinLock lock(g_scheduler_lock); if (pid != 0) peer = Thread::from_tid(pid); @@ -66,9 +73,6 @@ int Process::sys$sched_setparam(int pid, Userspace us if (!is_superuser() && m_euid != peer->process().m_uid && m_uid != peer->process().m_uid) return -EPERM; - if (desired_param.sched_priority < THREAD_PRIORITY_MIN || desired_param.sched_priority > THREAD_PRIORITY_MAX) - return -EINVAL; - peer->set_priority((u32)desired_param.sched_priority); return 0; } @@ -76,22 +80,27 @@ int Process::sys$sched_setparam(int pid, Userspace us int Process::sys$sched_getparam(pid_t pid, Userspace user_param) { REQUIRE_PROMISE(proc); - InterruptDisabler disabler; - auto* peer = Thread::current(); - if (pid != 0) { - // FIXME: PID/TID BUG - // The entire process is supposed to be affected. - peer = Thread::from_tid(pid); + int priority; + { + auto* peer = Thread::current(); + ScopedSpinLock lock(g_scheduler_lock); + if (pid != 0) { + // FIXME: PID/TID BUG + // The entire process is supposed to be affected. + peer = Thread::from_tid(pid); + } + + if (!peer) + return -ESRCH; + + if (!is_superuser() && m_euid != peer->process().m_uid && m_uid != peer->process().m_uid) + return -EPERM; + + priority = (int)peer->priority(); } - if (!peer) - return -ESRCH; - - if (!is_superuser() && m_euid != peer->process().m_uid && m_uid != peer->process().m_uid) - return -EPERM; - struct sched_param param { - (int)peer->priority() + priority }; if (!copy_to_user(user_param, ¶m)) return -EFAULT; @@ -103,8 +112,8 @@ int Process::sys$set_thread_boost(pid_t tid, int amount) REQUIRE_PROMISE(proc); if (amount < 0 || amount > 20) return -EINVAL; - InterruptDisabler disabler; - auto* thread = Thread::from_tid(tid); + ScopedSpinLock lock(g_scheduler_lock); + auto thread = Thread::from_tid(tid); if (!thread) return -ESRCH; if (thread->state() == Thread::State::Dead || thread->state() == Thread::State::Dying) diff --git a/Kernel/Syscalls/thread.cpp b/Kernel/Syscalls/thread.cpp index bd957da5df2..88b05b6aa5d 100644 --- a/Kernel/Syscalls/thread.cpp +++ b/Kernel/Syscalls/thread.cpp @@ -102,8 +102,7 @@ void Process::sys$exit_thread(Userspace exit_value) int Process::sys$detach_thread(pid_t tid) { REQUIRE_PROMISE(thread); - InterruptDisabler disabler; - auto* thread = Thread::from_tid(tid); + auto thread = Thread::from_tid(tid); if (!thread || thread->pid() != pid()) return -ESRCH; @@ -118,8 +117,7 @@ int Process::sys$join_thread(pid_t tid, Userspace exit_value) { REQUIRE_PROMISE(thread); - InterruptDisabler disabler; - auto* thread = Thread::from_tid(tid); + auto thread = Thread::from_tid(tid); if (!thread || thread->pid() != pid()) return -ESRCH; @@ -134,20 +132,14 @@ int Process::sys$join_thread(pid_t tid, Userspace exit_value) KResult try_join_result(KSuccess); auto result = current_thread->block(nullptr, *thread, try_join_result, joinee_exit_value); if (result == Thread::BlockResult::NotBlocked) { - ASSERT_INTERRUPTS_DISABLED(); if (try_join_result.is_error()) return try_join_result.error(); break; } - if (result == Thread::BlockResult::InterruptedByDeath) { - ASSERT_INTERRUPTS_DISABLED(); - break; - } + if (result == Thread::BlockResult::InterruptedByDeath) + return 0; // we're not going to return back to user mode } - // NOTE: 'thread' is very possibly deleted at this point. Clear it just to be safe. - thread = nullptr; - if (exit_value && !copy_to_user(exit_value, &joinee_exit_value)) return -EFAULT; return 0; @@ -164,8 +156,7 @@ int Process::sys$set_thread_name(pid_t tid, Userspace user_name, si if (name.length() > max_thread_name_size) return -EINVAL; - InterruptDisabler disabler; - auto* thread = Thread::from_tid(tid); + auto thread = Thread::from_tid(tid); if (!thread || thread->pid() != pid()) return -ESRCH; @@ -179,15 +170,16 @@ int Process::sys$get_thread_name(pid_t tid, Userspace buffer, size_t buff if (buffer_size == 0) return -EINVAL; - InterruptDisabler disabler; - auto* thread = Thread::from_tid(tid); + auto thread = Thread::from_tid(tid); if (!thread || thread->pid() != pid()) return -ESRCH; - if (thread->name().length() + 1 > (size_t)buffer_size) + // We must make a temporary copy here to avoid a race with sys$set_thread_name + auto thread_name = thread->name(); + if (thread_name.length() + 1 > (size_t)buffer_size) return -ENAMETOOLONG; - if (!copy_to_user(buffer, thread->name().characters(), thread->name().length() + 1)) + if (!copy_to_user(buffer, thread_name.characters(), thread_name.length() + 1)) return -EFAULT; return 0; } diff --git a/Kernel/Syscalls/waitid.cpp b/Kernel/Syscalls/waitid.cpp index f4d6f9c7ad2..9e75ba376e6 100644 --- a/Kernel/Syscalls/waitid.cpp +++ b/Kernel/Syscalls/waitid.cpp @@ -68,7 +68,9 @@ KResultOr Process::do_waitid(idtype_t idtype, int id, int options) return reap(*waitee_process); } else { // FIXME: PID/TID BUG - auto* waitee_thread = Thread::from_tid(waitee_pid.value()); + // Make sure to hold the scheduler lock so that we operate on a consistent state + ScopedSpinLock scheduler_lock(g_scheduler_lock); + auto waitee_thread = Thread::from_tid(waitee_pid.value()); if (!waitee_thread) return KResult(-ECHILD); ASSERT((options & WNOHANG) || waitee_thread->state() == Thread::State::Stopped); diff --git a/Kernel/Tasks/FinalizerTask.cpp b/Kernel/Tasks/FinalizerTask.cpp index c591a2bc841..fe82f84ab9f 100644 --- a/Kernel/Tasks/FinalizerTask.cpp +++ b/Kernel/Tasks/FinalizerTask.cpp @@ -31,7 +31,8 @@ namespace Kernel { void FinalizerTask::spawn() { - Process::create_kernel_process(g_finalizer, "FinalizerTask", [] { + RefPtr finalizer_thread; + Process::create_kernel_process(finalizer_thread, "FinalizerTask", [] { Thread::current()->set_priority(THREAD_PRIORITY_LOW); for (;;) { Thread::current()->wait_on(*g_finalizer_wait_queue, "FinalizerTask"); @@ -41,6 +42,7 @@ void FinalizerTask::spawn() Thread::finalize_dying_threads(); } }); + g_finalizer = finalizer_thread; } } diff --git a/Kernel/Tasks/SyncTask.cpp b/Kernel/Tasks/SyncTask.cpp index aea99872f1f..58c9e3c9332 100644 --- a/Kernel/Tasks/SyncTask.cpp +++ b/Kernel/Tasks/SyncTask.cpp @@ -33,7 +33,7 @@ namespace Kernel { void SyncTask::spawn() { - Thread* syncd_thread = nullptr; + RefPtr syncd_thread; Process::create_kernel_process(syncd_thread, "SyncTask", [] { dbg() << "SyncTask is running"; for (;;) { diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index caa53001335..43ebc169cea 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -100,17 +100,20 @@ Thread::Thread(NonnullRefPtr process) m_tss.esp0 = m_kernel_stack_top; } + // We need to add another reference if we could successfully create + // all the resources needed for this thread. The reason for this is that + // we don't want to delete this thread after dropping the reference, + // it may still be running or scheduled to be run. + // The finalizer is responsible for dropping this reference once this + // thread is ready to be cleaned up. + ref(); + if (m_process->pid() != 0) Scheduler::init_thread(*this); } Thread::~Thread() { - kfree_aligned(m_fpu_state); - - auto thread_cnt_before = m_process->m_thread_count.fetch_sub(1, AK::MemoryOrder::memory_order_acq_rel); - ASSERT(thread_cnt_before != 0); - ASSERT(!m_joiner); } @@ -269,10 +272,12 @@ const char* Thread::state_string() const return "Stopped"; case Thread::Queued: return "Queued"; - case Thread::Blocked: + case Thread::Blocked: { + ScopedSpinLock lock(m_lock); ASSERT(m_blocker != nullptr); return m_blocker->state_string(); } + } klog() << "Thread::state_string(): Invalid state: " << state(); ASSERT_NOT_REACHED(); return nullptr; @@ -295,6 +300,13 @@ void Thread::finalize() if (m_dump_backtrace_on_finalization) dbg() << backtrace_impl(); + + kfree_aligned(m_fpu_state); + + auto thread_cnt_before = m_process->m_thread_count.fetch_sub(1, AK::MemoryOrder::memory_order_acq_rel); + ASSERT(thread_cnt_before != 0); + if (thread_cnt_before == 1) + process().finalize(); } void Thread::finalize_dying_threads() @@ -310,11 +322,12 @@ void Thread::finalize_dying_threads() }); } for (auto* thread : dying_threads) { - auto& process = thread->process(); thread->finalize(); - delete thread; - if (process.m_thread_count.load(AK::MemoryOrder::memory_order_consume) == 0) - process.finalize(); + + // This thread will never execute again, drop the running reference + // NOTE: This may not necessarily drop the last reference if anything + // else is still holding onto this thread! + thread->unref(); } } @@ -762,9 +775,9 @@ KResultOr Thread::make_userspace_stack_for_main_thread(Vector argum return new_esp; } -Thread* Thread::clone(Process& process) +RefPtr Thread::clone(Process& process) { - auto* clone = new Thread(process); + auto clone = adopt(*new Thread(process)); memcpy(clone->m_signal_action_data, m_signal_action_data, sizeof(m_signal_action_data)); clone->m_signal_mask = m_signal_mask; memcpy(clone->m_fpu_state, m_fpu_state, sizeof(FPUState)); @@ -967,7 +980,7 @@ const LogStream& operator<<(const LogStream& stream, const Thread& value) return stream << value.process().name() << "(" << value.pid().value() << ":" << value.tid().value() << ")"; } -Thread::BlockResult Thread::wait_on(WaitQueue& queue, const char* reason, timeval* timeout, Atomic* lock, Thread* beneficiary) +Thread::BlockResult Thread::wait_on(WaitQueue& queue, const char* reason, timeval* timeout, Atomic* lock, RefPtr beneficiary) { auto* current_thread = Thread::current(); TimerId timer_id {}; @@ -1046,7 +1059,7 @@ Thread::BlockResult Thread::wait_on(WaitQueue& queue, const char* reason, timeva // Our thread was already removed from the queue. The only // way this can happen if someone else is trying to kill us. // In this case, the queue should not contain us anymore. - return BlockResult::InterruptedByDeath; + result = BlockResult::InterruptedByDeath; } // Make sure we cancel the timer if woke normally. @@ -1071,10 +1084,10 @@ void Thread::wake_from_queue() set_state(State::Running); } -Thread* Thread::from_tid(ThreadID tid) +RefPtr Thread::from_tid(ThreadID tid) { - InterruptDisabler disabler; - Thread* found_thread = nullptr; + RefPtr found_thread; + ScopedSpinLock lock(g_scheduler_lock); Thread::for_each([&](auto& thread) { if (thread.tid() == tid) { found_thread = &thread; @@ -1109,6 +1122,7 @@ void Thread::tracer_trap(const RegisterState& regs) const Thread::Blocker& Thread::blocker() const { + ASSERT(m_lock.own_lock()); ASSERT(m_blocker); return *m_blocker; } diff --git a/Kernel/Thread.h b/Kernel/Thread.h index d5cd017614f..57dc4cceb49 100644 --- a/Kernel/Thread.h +++ b/Kernel/Thread.h @@ -32,6 +32,8 @@ #include #include #include +#include +#include #include #include #include @@ -66,7 +68,9 @@ struct ThreadSpecificData { #define THREAD_AFFINITY_DEFAULT 0xffffffff -class Thread { +class Thread + : public RefCounted + , public Weakable { AK_MAKE_NONCOPYABLE(Thread); AK_MAKE_NONMOVABLE(Thread); @@ -82,7 +86,7 @@ public: explicit Thread(NonnullRefPtr); ~Thread(); - static Thread* from_tid(ThreadID); + static RefPtr from_tid(ThreadID); static void finalize_dying_threads(); ThreadID tid() const { return m_tid; } @@ -143,9 +147,23 @@ public: String backtrace(); Vector raw_backtrace(FlatPtr ebp, FlatPtr eip) const; - const String& name() const { return m_name; } - void set_name(const StringView& s) { m_name = s; } - void set_name(String&& name) { m_name = move(name); } + String name() const + { + // Because the name can be changed, we can't return a const + // reference here. We must make a copy + ScopedSpinLock lock(m_lock); + return m_name; + } + void set_name(const StringView& s) + { + ScopedSpinLock lock(m_lock); + m_name = s; + } + void set_name(String&& name) + { + ScopedSpinLock lock(m_lock); + m_name = move(name); + } void finalize(); @@ -452,7 +470,7 @@ public: return block(nullptr, state_string, move(condition)); } - BlockResult wait_on(WaitQueue& queue, const char* reason, timeval* timeout = nullptr, Atomic* lock = nullptr, Thread* beneficiary = nullptr); + BlockResult wait_on(WaitQueue& queue, const char* reason, timeval* timeout = nullptr, Atomic* lock = nullptr, RefPtr beneficiary = {}); void wake_from_queue(); void unblock(); @@ -578,7 +596,7 @@ public: return !m_is_joinable; } - Thread* clone(Process&); + RefPtr clone(Process&); template static IterationDecision for_each_in_state(State, Callback); diff --git a/Kernel/init.cpp b/Kernel/init.cpp index c0d317b9159..7a6751a32a2 100644 --- a/Kernel/init.cpp +++ b/Kernel/init.cpp @@ -168,8 +168,13 @@ extern "C" [[noreturn]] void init() Process::initialize(); Scheduler::initialize(); - Thread* init_stage2_thread = nullptr; - Process::create_kernel_process(init_stage2_thread, "init_stage2", init_stage2); + { + RefPtr init_stage2_thread; + Process::create_kernel_process(init_stage2_thread, "init_stage2", init_stage2); + // We need to make sure we drop the reference for init_stage2_thread + // before calling into Scheduler::start, otherwise we will have a + // dangling Thread that never gets cleaned up + } Scheduler::start(); ASSERT_NOT_REACHED(); @@ -351,7 +356,7 @@ void init_stage2() // FIXME: It would be nicer to set the mode from userspace. tty0->set_graphical(!text_mode); - Thread* thread = nullptr; + RefPtr thread; auto userspace_init = kernel_command_line().lookup("init").value_or("/bin/SystemServer"); Process::create_user_process(thread, userspace_init, (uid_t)0, (gid_t)0, ProcessID(0), error, {}, {}, tty0); if (error != 0) {