Kaynağa Gözat

Kernel: Remove ProcessInspectionHandle and make Process RefCounted

By making the Process class RefCounted we don't really need
ProcessInspectionHandle anymore. This also fixes some race
conditions where a Process may be deleted while still being
used by ProcFS.

Also make sure to acquire the Process' lock when accessing
regions.

Last but not least, there's no reason why a thread can't be
scheduled while being inspected, though in practice it won't
happen anyway because the scheduler lock is held at the same
time.
Tom 5 yıl önce
ebeveyn
işleme
538b985487

+ 118 - 117
Kernel/FileSystem/ProcFS.cpp

@@ -241,22 +241,21 @@ Optional<KBuffer> procfs$pid_fds(InodeIdentifier identifier)
     KBufferBuilder builder;
     JsonArraySerializer array { builder };
 
-    auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier));
-    if (!handle) {
+    auto process = Process::from_pid(to_pid(identifier));
+    if (!process) {
         array.finish();
         return builder.build();
     }
-    auto& process = handle->process();
-    if (process.number_of_open_file_descriptors() == 0) {
+    if (process->number_of_open_file_descriptors() == 0) {
         array.finish();
         return builder.build();
     }
 
-    for (int i = 0; i < process.max_open_file_descriptors(); ++i) {
-        auto description = process.file_description(i);
+    for (int i = 0; i < process->max_open_file_descriptors(); ++i) {
+        auto description = process->file_description(i);
         if (!description)
             continue;
-        bool cloexec = process.fd_flags(i) & FD_CLOEXEC;
+        bool cloexec = process->fd_flags(i) & FD_CLOEXEC;
 
         auto description_object = array.add_object();
         description_object.add("fd", i);
@@ -275,12 +274,11 @@ Optional<KBuffer> procfs$pid_fds(InodeIdentifier identifier)
 
 Optional<KBuffer> procfs$pid_fd_entry(InodeIdentifier identifier)
 {
-    auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier));
-    if (!handle)
+    auto process = Process::from_pid(to_pid(identifier));
+    if (!process)
         return {};
-    auto& process = handle->process();
     int fd = to_fd(identifier);
-    auto description = process.file_description(fd);
+    auto description = process->file_description(fd);
     if (!description)
         return {};
     return description->absolute_path().to_byte_buffer();
@@ -288,46 +286,48 @@ Optional<KBuffer> procfs$pid_fd_entry(InodeIdentifier identifier)
 
 Optional<KBuffer> procfs$pid_vm(InodeIdentifier identifier)
 {
-    auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier));
-    if (!handle)
+    auto process = Process::from_pid(to_pid(identifier));
+    if (!process)
         return {};
-    auto& process = handle->process();
     KBufferBuilder builder;
     JsonArraySerializer array { builder };
-    for (auto& region : process.regions()) {
-        if (!region.is_user_accessible() && !Process::current()->is_superuser())
-            continue;
-        auto region_object = array.add_object();
-        region_object.add("readable", region.is_readable());
-        region_object.add("writable", region.is_writable());
-        region_object.add("executable", region.is_executable());
-        region_object.add("stack", region.is_stack());
-        region_object.add("shared", region.is_shared());
-        region_object.add("user_accessible", region.is_user_accessible());
-        region_object.add("purgeable", region.vmobject().is_purgeable());
-        if (region.vmobject().is_purgeable()) {
-            region_object.add("volatile", static_cast<const PurgeableVMObject&>(region.vmobject()).is_volatile());
-        }
-        region_object.add("purgeable", region.vmobject().is_purgeable());
-        region_object.add("address", region.vaddr().get());
-        region_object.add("size", region.size());
-        region_object.add("amount_resident", region.amount_resident());
-        region_object.add("amount_dirty", region.amount_dirty());
-        region_object.add("cow_pages", region.cow_pages());
-        region_object.add("name", region.name());
-        region_object.add("vmobject", region.vmobject().class_name());
-
-        StringBuilder pagemap_builder;
-        for (size_t i = 0; i < region.page_count(); ++i) {
-            auto* page = region.physical_page(i);
-            if (!page)
-                pagemap_builder.append('N');
-            else if (page->is_shared_zero_page())
-                pagemap_builder.append('Z');
-            else
-                pagemap_builder.append('P');
+    {
+        ScopedSpinLock lock(process->get_lock());
+        for (auto& region : process->regions()) {
+            if (!region.is_user_accessible() && !Process::current()->is_superuser())
+                continue;
+            auto region_object = array.add_object();
+            region_object.add("readable", region.is_readable());
+            region_object.add("writable", region.is_writable());
+            region_object.add("executable", region.is_executable());
+            region_object.add("stack", region.is_stack());
+            region_object.add("shared", region.is_shared());
+            region_object.add("user_accessible", region.is_user_accessible());
+            region_object.add("purgeable", region.vmobject().is_purgeable());
+            if (region.vmobject().is_purgeable()) {
+                region_object.add("volatile", static_cast<const PurgeableVMObject&>(region.vmobject()).is_volatile());
+            }
+            region_object.add("purgeable", region.vmobject().is_purgeable());
+            region_object.add("address", region.vaddr().get());
+            region_object.add("size", region.size());
+            region_object.add("amount_resident", region.amount_resident());
+            region_object.add("amount_dirty", region.amount_dirty());
+            region_object.add("cow_pages", region.cow_pages());
+            region_object.add("name", region.name());
+            region_object.add("vmobject", region.vmobject().class_name());
+	    
+            StringBuilder pagemap_builder;
+            for (size_t i = 0; i < region.page_count(); ++i) {
+                auto* page = region.physical_page(i);
+                if (!page)
+                    pagemap_builder.append('N');
+                else if (page->is_shared_zero_page())
+                    pagemap_builder.append('Z');
+                else
+                    pagemap_builder.append('P');
+            }
+            region_object.add("pagemap", pagemap_builder.to_string());
         }
-        region_object.add("pagemap", pagemap_builder.to_string());
     }
     array.finish();
     return builder.build();
@@ -557,46 +557,47 @@ Optional<KBuffer> procfs$net_local(InodeIdentifier)
 
 Optional<KBuffer> procfs$pid_vmobjects(InodeIdentifier identifier)
 {
-    auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier));
-    if (!handle)
+    auto process = Process::from_pid(to_pid(identifier));
+    if (!process)
         return {};
-    auto& process = handle->process();
     KBufferBuilder builder;
     builder.appendf("BEGIN       END         SIZE        NAME\n");
-    for (auto& region : process.regions()) {
-        builder.appendf("%x -- %x    %x    %s\n",
-            region.vaddr().get(),
-            region.vaddr().offset(region.size() - 1).get(),
-            region.size(),
-            region.name().characters());
-        builder.appendf("VMO: %s @ %x(%u)\n",
-            region.vmobject().is_anonymous() ? "anonymous" : "file-backed",
-            &region.vmobject(),
-            region.vmobject().ref_count());
-        for (size_t i = 0; i < region.vmobject().page_count(); ++i) {
-            auto& physical_page = region.vmobject().physical_pages()[i];
-            bool should_cow = false;
-            if (i >= region.first_page_index() && i <= region.last_page_index())
-                should_cow = region.should_cow(i - region.first_page_index());
-            builder.appendf("P%x%s(%u) ",
-                physical_page ? physical_page->paddr().get() : 0,
-                should_cow ? "!" : "",
-                physical_page ? physical_page->ref_count() : 0);
+    {
+        ScopedSpinLock lock(process->get_lock());
+        for (auto& region : process->regions()) {
+            builder.appendf("%x -- %x    %x    %s\n",
+                region.vaddr().get(),
+                region.vaddr().offset(region.size() - 1).get(),
+                region.size(),
+                region.name().characters());
+            builder.appendf("VMO: %s @ %x(%u)\n",
+                region.vmobject().is_anonymous() ? "anonymous" : "file-backed",
+                &region.vmobject(),
+                region.vmobject().ref_count());
+            for (size_t i = 0; i < region.vmobject().page_count(); ++i) {
+                auto& physical_page = region.vmobject().physical_pages()[i];
+                bool should_cow = false;
+                if (i >= region.first_page_index() && i <= region.last_page_index())
+                    should_cow = region.should_cow(i - region.first_page_index());
+                builder.appendf("P%x%s(%u) ",
+                    physical_page ? physical_page->paddr().get() : 0,
+                    should_cow ? "!" : "",
+                    physical_page ? physical_page->ref_count() : 0);
+            }
+            builder.appendf("\n");
         }
-        builder.appendf("\n");
     }
     return builder.build();
 }
 
 Optional<KBuffer> procfs$pid_unveil(InodeIdentifier identifier)
 {
-    auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier));
-    if (!handle)
+    auto process = Process::from_pid(to_pid(identifier));
+    if (!process)
         return {};
-    auto& process = handle->process();
     KBufferBuilder builder;
     JsonArraySerializer array { builder };
-    for (auto& unveiled_path : process.unveiled_paths()) {
+    for (auto& unveiled_path : process->unveiled_paths()) {
         auto obj = array.add_object();
         obj.add("path", unveiled_path.path);
         StringBuilder permissions_builder;
@@ -616,38 +617,36 @@ Optional<KBuffer> procfs$pid_unveil(InodeIdentifier identifier)
 
 Optional<KBuffer> procfs$pid_stack(InodeIdentifier identifier)
 {
-    auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier));
-    if (!handle)
+    auto process = Process::from_pid(to_pid(identifier));
+    if (!process)
         return {};
-    auto& process = handle->process();
-    return process.backtrace(*handle);
+    return process->backtrace();
 }
 
 Optional<KBuffer> procfs$pid_exe(InodeIdentifier identifier)
 {
-    auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier));
-    if (!handle)
+    auto process = Process::from_pid(to_pid(identifier));
+    if (!process)
         return {};
-    auto& process = handle->process();
-    auto* custody = process.executable();
+    auto* custody = process->executable();
     ASSERT(custody);
     return custody->absolute_path().to_byte_buffer();
 }
 
 Optional<KBuffer> procfs$pid_cwd(InodeIdentifier identifier)
 {
-    auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier));
-    if (!handle)
+    auto process = Process::from_pid(to_pid(identifier));
+    if (!process)
         return {};
-    return handle->process().current_directory().absolute_path().to_byte_buffer();
+    return process->current_directory().absolute_path().to_byte_buffer();
 }
 
 Optional<KBuffer> procfs$pid_root(InodeIdentifier identifier)
 {
-    auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier));
-    if (!handle)
+    auto process = Process::from_pid(to_pid(identifier));
+    if (!process)
         return {};
-    return handle->process().root_directory_relative_to_global_root().absolute_path().to_byte_buffer();
+    return process->root_directory_relative_to_global_root().absolute_path().to_byte_buffer();
 }
 
 Optional<KBuffer> procfs$self(InodeIdentifier)
@@ -783,8 +782,6 @@ Optional<KBuffer> procfs$memstat(InodeIdentifier)
 
 Optional<KBuffer> procfs$all(InodeIdentifier)
 {
-    ScopedSpinLock lock(g_scheduler_lock);
-    auto processes = Process::all_processes();
     KBufferBuilder builder;
     JsonArraySerializer array { builder };
 
@@ -856,9 +853,12 @@ Optional<KBuffer> procfs$all(InodeIdentifier)
             return IterationDecision::Continue;
         });
     };
+
+    ScopedSpinLock lock(g_scheduler_lock);
+    auto processes = Process::all_processes();
     build_process(*Scheduler::colonel());
-    for (auto* process : processes)
-        build_process(*process);
+    for (auto& process : processes)
+        build_process(process);
     array.finish();
     return builder.build();
 }
@@ -1069,9 +1069,15 @@ InodeMetadata ProcFSInode::metadata() const
 #endif
 
     if (is_process_related_file(identifier())) {
-        auto handle = ProcessInspectionHandle::from_pid(pid);
-        metadata.uid = handle->process().sys$getuid();
-        metadata.gid = handle->process().sys$getgid();
+        auto process = Process::from_pid(pid);
+        if (process) {
+            metadata.uid = process->sys$getuid();
+            metadata.gid = process->sys$getgid();
+        } else {
+            // TODO: How to handle this?
+            metadata.uid = 0;
+            metadata.gid = 0;
+        }
     }
 
     if (proc_parent_directory == PDI_PID_fd) {
@@ -1232,13 +1238,12 @@ KResult ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntr
         break;
 
     case FI_PID: {
-        auto handle = ProcessInspectionHandle::from_pid(pid);
-        if (!handle)
+        auto process = Process::from_pid(pid);
+        if (!process)
             return KResult(-ENOENT);
-        auto& process = handle->process();
         for (auto& entry : fs().m_entries) {
             if (entry.proc_file_type > __FI_PID_Start && entry.proc_file_type < __FI_PID_End) {
-                if (entry.proc_file_type == FI_PID_exe && !process.executable())
+                if (entry.proc_file_type == FI_PID_exe && !process->executable())
                     continue;
                 // FIXME: strlen() here is sad.
                 callback({ entry.name, strlen(entry.name), to_identifier(fsid(), PDI_PID, pid, (ProcFileType)entry.proc_file_type), 0 });
@@ -1247,12 +1252,11 @@ KResult ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntr
     } break;
 
     case FI_PID_fd: {
-        auto handle = ProcessInspectionHandle::from_pid(pid);
-        if (!handle)
+        auto process = Process::from_pid(pid);
+        if (!process)
             return KResult(-ENOENT);
-        auto& process = handle->process();
-        for (int i = 0; i < process.max_open_file_descriptors(); ++i) {
-            auto description = process.file_description(i);
+        for (int i = 0; i < process->max_open_file_descriptors(); ++i) {
+            auto description = process->file_description(i);
             if (!description)
                 continue;
             char name[16];
@@ -1324,13 +1328,12 @@ RefPtr<Inode> ProcFSInode::lookup(StringView name)
     }
 
     if (proc_file_type == FI_PID) {
-        auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier()));
-        if (!handle)
+        auto process = Process::from_pid(to_pid(identifier()));
+        if (!process)
             return {};
-        auto& process = handle->process();
         for (auto& entry : fs().m_entries) {
             if (entry.proc_file_type > __FI_PID_Start && entry.proc_file_type < __FI_PID_End) {
-                if (entry.proc_file_type == FI_PID_exe && !process.executable())
+                if (entry.proc_file_type == FI_PID_exe && !process->executable())
                     continue;
                 if (entry.name == nullptr)
                     continue;
@@ -1348,8 +1351,7 @@ RefPtr<Inode> ProcFSInode::lookup(StringView name)
             return {};
         bool fd_exists = false;
         {
-            InterruptDisabler disabler;
-            if (auto* process = Process::from_pid(to_pid(identifier())))
+            if (auto process = Process::from_pid(to_pid(identifier())))
                 fd_exists = process->file_description(name_as_number.value());
         }
         if (fd_exists)
@@ -1415,16 +1417,15 @@ KResultOr<NonnullRefPtr<Custody>> ProcFSInode::resolve_as_link(Custody& base, Re
 
     auto pid = to_pid(identifier());
     auto proc_file_type = to_proc_file_type(identifier());
-    auto handle = ProcessInspectionHandle::from_pid(pid);
-    if (!handle)
+    auto process = Process::from_pid(pid);
+    if (!process)
         return KResult(-ENOENT);
-    auto& process = handle->process();
 
     if (to_proc_parent_directory(identifier()) == PDI_PID_fd) {
         if (out_parent)
             *out_parent = base;
         int fd = to_fd(identifier());
-        auto description = process.file_description(fd);
+        auto description = process->file_description(fd);
         if (!description)
             return KResult(-ENOENT);
         auto proxy_inode = ProcFSProxyInode::create(const_cast<ProcFS&>(fs()), *description);
@@ -1435,16 +1436,16 @@ KResultOr<NonnullRefPtr<Custody>> ProcFSInode::resolve_as_link(Custody& base, Re
 
     switch (proc_file_type) {
     case FI_PID_cwd:
-        res = &process.current_directory();
+        res = &process->current_directory();
         break;
     case FI_PID_exe:
-        res = process.executable();
+        res = process->executable();
         break;
     case FI_PID_root:
         // Note: we open root_directory() here, not
         // root_directory_relative_to_global_root().
         // This seems more useful.
-        res = &process.root_directory();
+        res = &process->root_directory();
         break;
     default:
         ASSERT_NOT_REACHED();

+ 17 - 18
Kernel/Process.cpp

@@ -122,13 +122,13 @@ Vector<pid_t> Process::all_pids()
     return pids;
 }
 
-Vector<Process*> Process::all_processes()
+NonnullRefPtrVector<Process> Process::all_processes()
 {
-    Vector<Process*> processes;
+    NonnullRefPtrVector<Process> processes;
     ScopedSpinLock lock(g_processes_lock);
     processes.ensure_capacity((int)g_processes->size_slow());
     for (auto& process : *g_processes)
-        processes.append(&process);
+        processes.append(NonnullRefPtr<Process>(process));
     return processes;
 }
 
@@ -286,7 +286,7 @@ void Process::kill_all_threads()
     });
 }
 
-Process* Process::create_user_process(Thread*& first_thread, const String& path, uid_t uid, gid_t gid, pid_t parent_pid, int& error, Vector<String>&& arguments, Vector<String>&& environment, TTY* tty)
+RefPtr<Process> Process::create_user_process(Thread*& first_thread, const String& path, uid_t uid, gid_t gid, pid_t parent_pid, int& error, Vector<String>&& arguments, Vector<String>&& environment, TTY* tty)
 {
     auto parts = path.split('/');
     if (arguments.is_empty()) {
@@ -296,7 +296,7 @@ Process* Process::create_user_process(Thread*& first_thread, const String& path,
     RefPtr<Custody> root;
     {
         ScopedSpinLock lock(g_processes_lock);
-        if (auto* parent = Process::from_pid(parent_pid)) {
+        if (auto parent = Process::from_pid(parent_pid)) {
             cwd = parent->m_cwd;
             root = parent->m_root_directory;
         }
@@ -308,7 +308,7 @@ Process* Process::create_user_process(Thread*& first_thread, const String& path,
     if (!root)
         root = VFS::the().root_custody();
 
-    auto* process = new Process(first_thread, parts.take_last(), uid, gid, parent_pid, Ring3, move(cwd), nullptr, tty);
+    auto process = adopt(*new Process(first_thread, parts.take_last(), uid, gid, parent_pid, Ring3, move(cwd), nullptr, tty));
     process->m_fds.resize(m_max_open_file_descriptors);
     auto& device_to_use_as_tty = tty ? (CharacterDevice&)*tty : NullDevice::the();
     auto description = device_to_use_as_tty.open(O_RDWR).value();
@@ -320,26 +320,27 @@ Process* Process::create_user_process(Thread*& first_thread, const String& path,
     if (error != 0) {
         dbg() << "Failed to exec " << path << ": " << error;
         delete first_thread;
-        delete process;
-        return nullptr;
+        return {};
     }
 
     {
         ScopedSpinLock lock(g_processes_lock);
         g_processes->prepend(process);
+        process->ref();
     }
     error = 0;
     return process;
 }
 
-Process* Process::create_kernel_process(Thread*& first_thread, String&& name, void (*e)(), u32 affinity)
+NonnullRefPtr<Process> Process::create_kernel_process(Thread*& first_thread, String&& name, void (*e)(), u32 affinity)
 {
-    auto* process = new Process(first_thread, move(name), (uid_t)0, (gid_t)0, (pid_t)0, Ring0);
+    auto process = adopt(*new Process(first_thread, move(name), (uid_t)0, (gid_t)0, (pid_t)0, Ring0));
     first_thread->tss().eip = (FlatPtr)e;
 
     if (process->pid() != 0) {
         ScopedSpinLock lock(g_processes_lock);
         g_processes->prepend(process);
+        process->ref();
     }
 
     first_thread->set_affinity(affinity);
@@ -472,15 +473,14 @@ void Process::crash(int signal, u32 eip, bool out_of_memory)
     ASSERT_NOT_REACHED();
 }
 
-Process* Process::from_pid(pid_t pid)
+RefPtr<Process> Process::from_pid(pid_t pid)
 {
-    ASSERT_INTERRUPTS_DISABLED();
     ScopedSpinLock lock(g_processes_lock);
     for (auto& process : *g_processes) {
         if (process.pid() == pid)
             return &process;
     }
-    return nullptr;
+    return {};
 }
 
 RefPtr<FileDescription> Process::file_description(int fd) const
@@ -567,7 +567,7 @@ siginfo_t Process::reap(Process& process)
     ASSERT(g_processes_lock.is_locked());
 
     if (process.ppid()) {
-        auto* parent = Process::from_pid(process.ppid());
+        auto parent = Process::from_pid(process.ppid());
         if (parent) {
             parent->m_ticks_in_user_for_dead_children += process.m_ticks_in_user + process.m_ticks_in_user_for_dead_children;
             parent->m_ticks_in_kernel_for_dead_children += process.m_ticks_in_kernel + process.m_ticks_in_kernel_for_dead_children;
@@ -579,8 +579,7 @@ siginfo_t Process::reap(Process& process)
 #endif
     ASSERT(process.is_dead());
     g_processes->remove(&process);
-
-    delete &process;
+    process.unref();
     return siginfo;
 }
 
@@ -822,12 +821,12 @@ void Process::FileDescriptionAndFlags::set(NonnullRefPtr<FileDescription>&& desc
     m_flags = flags;
 }
 
-KBuffer Process::backtrace(ProcessInspectionHandle& handle) const
+KBuffer Process::backtrace() const
 {
     KBufferBuilder builder;
     for_each_thread([&](Thread& thread) {
         builder.appendf("Thread %d (%s):\n", thread.tid(), thread.name().characters());
-        builder.append(thread.backtrace(handle));
+        builder.append(thread.backtrace());
         return IterationDecision::Continue;
     });
     return builder.build();

+ 16 - 61
Kernel/Process.h

@@ -31,6 +31,7 @@
 #include <AK/HashMap.h>
 #include <AK/InlineLinkedList.h>
 #include <AK/NonnullOwnPtrVector.h>
+#include <AK/NonnullRefPtrVector.h>
 #include <AK/String.h>
 #include <AK/Userspace.h>
 #include <AK/WeakPtr.h>
@@ -105,7 +106,7 @@ struct UnveiledPath {
     unsigned permissions { 0 };
 };
 
-class Process : public InlineLinkedListNode<Process> {
+class Process : public RefCounted<Process>, public InlineLinkedListNode<Process> {
     AK_MAKE_NONCOPYABLE(Process);
     AK_MAKE_NONMOVABLE(Process);
 
@@ -119,12 +120,12 @@ public:
         return current_thread ? &current_thread->process() : nullptr;
     }
 
-    static Process* create_kernel_process(Thread*& first_thread, String&& name, void (*entry)(), u32 affinity = THREAD_AFFINITY_DEFAULT);
-    static Process* create_user_process(Thread*& first_thread, const String& path, uid_t, gid_t, pid_t ppid, int& error, Vector<String>&& arguments = Vector<String>(), Vector<String>&& environment = Vector<String>(), TTY* = nullptr);
+    static NonnullRefPtr<Process> create_kernel_process(Thread*& first_thread, String&& name, void (*entry)(), u32 affinity = THREAD_AFFINITY_DEFAULT);
+    static RefPtr<Process> create_user_process(Thread*& first_thread, const String& path, uid_t, gid_t, pid_t ppid, int& error, Vector<String>&& arguments = Vector<String>(), Vector<String>&& environment = Vector<String>(), TTY* = nullptr);
     ~Process();
 
     static Vector<pid_t> all_pids();
-    static Vector<Process*> all_processes();
+    static AK::NonnullRefPtrVector<Process> all_processes();
 
     Thread* create_kernel_thread(void (*entry)(), u32 priority, const String& name, u32 affinity = THREAD_AFFINITY_DEFAULT, bool joinable = true);
 
@@ -136,7 +137,7 @@ public:
         Ring3 = 3,
     };
 
-    KBuffer backtrace(ProcessInspectionHandle&) const;
+    KBuffer backtrace() const;
 
     bool is_dead() const { return m_dead; }
 
@@ -146,7 +147,7 @@ public:
     PageDirectory& page_directory() { return *m_page_directory; }
     const PageDirectory& page_directory() const { return *m_page_directory; }
 
-    static Process* from_pid(pid_t);
+    static RefPtr<Process> from_pid(pid_t);
 
     const String& name() const { return m_name; }
     pid_t pid() const { return m_pid; }
@@ -182,6 +183,8 @@ public:
     void die();
     void finalize();
 
+    ALWAYS_INLINE SpinLock<u32>& get_lock() const { return m_lock; }
+
     int sys$yield();
     int sys$sync();
     int sys$beep();
@@ -343,7 +346,11 @@ public:
     void set_tty(TTY*);
 
     size_t region_count() const { return m_regions.size(); }
-    const NonnullOwnPtrVector<Region>& regions() const { return m_regions; }
+    const NonnullOwnPtrVector<Region>& regions() const
+    {
+        ASSERT(m_lock.is_locked());
+        return m_regions;
+    }
     void dump_regions();
 
     u32 m_ticks_in_user { 0 };
@@ -485,11 +492,6 @@ public:
     Region& allocate_split_region(const Region& source_region, const Range&, size_t offset_in_vmobject);
     Vector<Region*, 2> split_region_around_range(const Region& source_region, const Range&);
 
-    bool is_being_inspected() const
-    {
-        return m_inspector_count;
-    }
-
     void terminate_due_to_signal(u8 signal);
     KResult send_signal(u8 signal, Process* sender);
 
@@ -541,15 +543,6 @@ public:
         return m_unveiled_paths;
     }
 
-    void increment_inspector_count(Badge<ProcessInspectionHandle>)
-    {
-        ++m_inspector_count;
-    }
-    void decrement_inspector_count(Badge<ProcessInspectionHandle>)
-    {
-        --m_inspector_count;
-    }
-
     bool wait_for_tracer_at_next_execve() const
     {
         return m_wait_for_tracer_at_next_execve;
@@ -699,50 +692,12 @@ private:
 
     OwnPtr<PerformanceEventBuffer> m_perf_event_buffer;
 
-    u32 m_inspector_count { 0 };
-
     // This member is used in the implementation of ptrace's PT_TRACEME flag.
     // If it is set to true, the process will stop at the next execve syscall
     // and wait for a tracer to attach.
     bool m_wait_for_tracer_at_next_execve { false };
 };
 
-class ProcessInspectionHandle {
-public:
-    ProcessInspectionHandle(Process& process)
-        : m_process(process)
-    {
-        if (&process != Process::current()) {
-            InterruptDisabler disabler;
-            m_process.increment_inspector_count({});
-        }
-    }
-    ~ProcessInspectionHandle()
-    {
-        if (&m_process != Process::current()) {
-            InterruptDisabler disabler;
-            m_process.decrement_inspector_count({});
-        }
-    }
-
-    Process& process() { return m_process; }
-
-    static OwnPtr<ProcessInspectionHandle> from_pid(pid_t pid)
-    {
-        InterruptDisabler disabler;
-        auto* process = Process::from_pid(pid);
-        if (process)
-            return make<ProcessInspectionHandle>(*process);
-        return nullptr;
-    }
-
-    Process* operator->() { return &m_process; }
-    Process& operator*() { return m_process; }
-
-private:
-    Process& m_process;
-};
-
 extern InlineLinkedList<Process>* g_processes;
 extern RecursiveSpinLock g_processes_lock;
 
@@ -833,7 +788,7 @@ inline bool InodeMetadata::may_execute(const Process& process) const
 
 inline int Thread::pid() const
 {
-    return m_process.pid();
+    return m_process->pid();
 }
 
 inline const LogStream& operator<<(const LogStream& stream, const Process& process)
@@ -843,7 +798,7 @@ inline const LogStream& operator<<(const LogStream& stream, const Process& proce
 
 inline u32 Thread::effective_priority() const
 {
-    return m_priority + m_process.priority_boost() + m_priority_boost + m_extra_priority;
+    return m_priority + m_process->priority_boost() + m_priority_boost + m_extra_priority;
 }
 
 #define REQUIRE_NO_PROMISES                        \

+ 5 - 7
Kernel/Scheduler.cpp

@@ -66,6 +66,7 @@ void Scheduler::update_state_for_thread(Thread& thread)
 {
     ASSERT_INTERRUPTS_DISABLED();
     ASSERT(g_scheduler_data);
+    ASSERT(g_scheduler_lock.own_lock());
     auto& list = g_scheduler_data->thread_list_for_state(thread.state());
 
     if (list.contains(thread))
@@ -255,7 +256,7 @@ bool Thread::WaitBlocker::should_unblock(Thread& thread, time_t, long)
 {
     bool should_unblock = m_wait_options & WNOHANG;
     if (m_waitee_pid != -1) {
-        auto* peer = Process::from_pid(m_waitee_pid);
+        auto peer = Process::from_pid(m_waitee_pid);
         if (!peer)
             return true;
     }
@@ -458,9 +459,6 @@ bool Scheduler::pick_next()
     Thread* thread_to_schedule = nullptr;
 
     for (auto* thread : sorted_runnables) {
-        if (thread->process().is_being_inspected())
-            continue;
-
         if (thread->process().exec_tid() && thread->process().exec_tid() != thread->tid())
             continue;
 
@@ -516,6 +514,8 @@ bool Scheduler::yield()
 
 bool Scheduler::donate_to(Thread* beneficiary, const char* reason)
 {
+    ASSERT(beneficiary);
+    
     // Set the m_in_scheduler flag before acquiring the spinlock. This
     // prevents a recursive call into Scheduler::invoke_async upon
     // leaving the scheduler lock.
@@ -534,8 +534,6 @@ bool Scheduler::donate_to(Thread* beneficiary, const char* reason)
     ScopedSpinLock lock(g_scheduler_lock);
 
     ASSERT(!proc.in_irq());
-    if (!Thread::is_thread(beneficiary))
-        return false;
 
     if (proc.in_critical()) {
         proc.invoke_scheduler_async();
@@ -660,7 +658,7 @@ void Scheduler::initialize()
     g_finalizer_wait_queue = new WaitQueue;
 
     g_finalizer_has_work.store(false, AK::MemoryOrder::memory_order_release);
-    s_colonel_process = Process::create_kernel_process(idle_thread, "colonel", idle_loop, 1);
+    s_colonel_process = &Process::create_kernel_process(idle_thread, "colonel", idle_loop, 1).leak_ref();
     ASSERT(s_colonel_process);
     ASSERT(idle_thread);
     idle_thread->set_priority(THREAD_PRIORITY_MIN);

+ 1 - 1
Kernel/Syscalls/kill.cpp

@@ -139,7 +139,7 @@ int Process::sys$kill(pid_t pid, int signal)
         return do_killself(signal);
     }
     ScopedSpinLock lock(g_processes_lock);
-    auto* peer = Process::from_pid(pid);
+    auto peer = Process::from_pid(pid);
     if (!peer)
         return -ESRCH;
     return do_kill(*peer, signal);

+ 2 - 2
Kernel/Syscalls/profiling.cpp

@@ -33,7 +33,7 @@ int Process::sys$profiling_enable(pid_t pid)
 {
     REQUIRE_NO_PROMISES;
     ScopedSpinLock lock(g_processes_lock);
-    auto* process = Process::from_pid(pid);
+    auto process = Process::from_pid(pid);
     if (!process)
         return -ESRCH;
     if (process->is_dead())
@@ -48,7 +48,7 @@ int Process::sys$profiling_enable(pid_t pid)
 int Process::sys$profiling_disable(pid_t pid)
 {
     ScopedSpinLock lock(g_processes_lock);
-    auto* process = Process::from_pid(pid);
+    auto process = Process::from_pid(pid);
     if (!process)
         return -ESRCH;
     if (!is_superuser() && process->uid() != m_uid)

+ 1 - 1
Kernel/Syscalls/sched.cpp

@@ -121,7 +121,7 @@ int Process::sys$set_process_boost(pid_t pid, int amount)
     if (amount < 0 || amount > 20)
         return -EINVAL;
     ScopedSpinLock lock(g_processes_lock);
-    auto* process = Process::from_pid(pid);
+    auto process = Process::from_pid(pid);
     if (!process || process->is_dead())
         return -ESRCH;
     if (!is_superuser() && process->uid() != euid())

+ 4 - 4
Kernel/Syscalls/setpgid.cpp

@@ -35,7 +35,7 @@ pid_t Process::sys$getsid(pid_t pid)
     if (pid == 0)
         return m_sid;
     ScopedSpinLock lock(g_processes_lock);
-    auto* process = Process::from_pid(pid);
+    auto process = Process::from_pid(pid);
     if (!process)
         return -ESRCH;
     if (m_sid != process->m_sid)
@@ -66,7 +66,7 @@ pid_t Process::sys$getpgid(pid_t pid)
     if (pid == 0)
         return m_pgid;
     ScopedSpinLock lock(g_processes_lock); // FIXME: Use a ProcessHandle
-    auto* process = Process::from_pid(pid);
+    auto process = Process::from_pid(pid);
     if (!process)
         return -ESRCH;
     return process->m_pgid;
@@ -81,7 +81,7 @@ pid_t Process::sys$getpgrp()
 static pid_t get_sid_from_pgid(pid_t pgid)
 {
     ScopedSpinLock lock(g_processes_lock);
-    auto* group_leader = Process::from_pid(pgid);
+    auto group_leader = Process::from_pid(pgid);
     if (!group_leader)
         return -1;
     return group_leader->sid();
@@ -96,7 +96,7 @@ int Process::sys$setpgid(pid_t specified_pid, pid_t specified_pgid)
         // The value of the pgid argument is less than 0, or is not a value supported by the implementation.
         return -EINVAL;
     }
-    auto* process = Process::from_pid(pid);
+    auto process = Process::from_pid(pid);
     if (!process)
         return -ESRCH;
     if (process != this && process->ppid() != m_pid) {

+ 1 - 1
Kernel/Syscalls/shbuf.cpp

@@ -81,7 +81,7 @@ int Process::sys$shbuf_allow_pid(int shbuf_id, pid_t peer_pid)
         return -EPERM;
     {
         ScopedSpinLock lock(g_processes_lock);
-        auto* peer = Process::from_pid(peer_pid);
+        auto peer = Process::from_pid(peer_pid);
         if (!peer)
             return -ESRCH;
     }

+ 1 - 1
Kernel/Syscalls/waitid.cpp

@@ -54,7 +54,7 @@ KResultOr<siginfo_t> Process::do_waitid(idtype_t idtype, int id, int options)
     ScopedSpinLock lock(g_processes_lock);
 
     // NOTE: If waitee was -1, m_waitee_pid will have been filled in by the scheduler.
-    Process* waitee_process = Process::from_pid(waitee_pid);
+    auto waitee_process = Process::from_pid(waitee_pid);
     if (!waitee_process)
         return KResult(-ECHILD);
 

+ 1 - 1
Kernel/TTY/TTY.cpp

@@ -303,7 +303,7 @@ int TTY::ioctl(FileDescription&, unsigned request, FlatPtr arg)
             return -EINVAL;
         {
             InterruptDisabler disabler;
-            auto* process = Process::from_pid(pgid);
+            auto process = Process::from_pid(pgid);
             if (!process)
                 return -EPERM;
             if (pgid != process->pgid())

+ 17 - 49
Kernel/Thread.cpp

@@ -46,27 +46,18 @@
 
 namespace Kernel {
 
-HashTable<Thread*>& thread_table()
+Thread::Thread(NonnullRefPtr<Process> process)
+    : m_process(move(process))
+    , m_name(m_process->name())
 {
-    ASSERT_INTERRUPTS_DISABLED();
-    static HashTable<Thread*>* table;
-    if (!table)
-        table = new HashTable<Thread*>;
-    return *table;
-}
-
-Thread::Thread(Process& process)
-    : m_process(process)
-    , m_name(process.name())
-{
-    if (m_process.m_thread_count.fetch_add(1, AK::MemoryOrder::memory_order_acq_rel) == 0) {
+    if (m_process->m_thread_count.fetch_add(1, AK::MemoryOrder::memory_order_acq_rel) == 0) {
         // First thread gets TID == PID
-        m_tid = process.pid();
+        m_tid = m_process->pid();
     } else {
         m_tid = Process::allocate_pid();
     }
 #ifdef THREAD_DEBUG
-    dbg() << "Created new thread " << process.name() << "(" << process.pid() << ":" << m_tid << ")";
+    dbg() << "Created new thread " << m_process->name() << "(" << m_process->pid() << ":" << m_tid << ")";
 #endif
     set_default_signal_dispositions();
     m_fpu_state = (FPUState*)kmalloc_aligned(sizeof(FPUState), 16);
@@ -77,7 +68,7 @@ Thread::Thread(Process& process)
     // Only IF is set when a process boots.
     m_tss.eflags = 0x0202;
 
-    if (m_process.is_ring0()) {
+    if (m_process->is_ring0()) {
         m_tss.cs = GDT_SELECTOR_CODE0;
         m_tss.ds = GDT_SELECTOR_DATA0;
         m_tss.es = GDT_SELECTOR_DATA0;
@@ -93,14 +84,14 @@ Thread::Thread(Process& process)
         m_tss.gs = GDT_SELECTOR_TLS | 3;
     }
 
-    m_tss.cr3 = m_process.page_directory().cr3();
+    m_tss.cr3 = m_process->page_directory().cr3();
 
     m_kernel_stack_region = MM.allocate_kernel_region(default_kernel_stack_size, String::format("Kernel Stack (Thread %d)", m_tid), Region::Access::Read | Region::Access::Write, false, true);
     m_kernel_stack_region->set_stack(true);
     m_kernel_stack_base = m_kernel_stack_region->vaddr().get();
     m_kernel_stack_top = m_kernel_stack_region->vaddr().offset(default_kernel_stack_size).get() & 0xfffffff8u;
 
-    if (m_process.is_ring0()) {
+    if (m_process->is_ring0()) {
         m_tss.esp = m_tss.esp0 = m_kernel_stack_top;
     } else {
         // Ring 3 processes get a separate stack for ring 0.
@@ -109,22 +100,15 @@ Thread::Thread(Process& process)
         m_tss.esp0 = m_kernel_stack_top;
     }
 
-    if (m_process.pid() != 0) {
-        InterruptDisabler disabler;
-        thread_table().set(this);
+    if (m_process->pid() != 0)
         Scheduler::init_thread(*this);
-    }
 }
 
 Thread::~Thread()
 {
     kfree_aligned(m_fpu_state);
-    {
-        InterruptDisabler disabler;
-        thread_table().remove(this);
-    }
 
-    auto thread_cnt_before = m_process.m_thread_count.fetch_sub(1, AK::MemoryOrder::memory_order_acq_rel);
+    auto thread_cnt_before = m_process->m_thread_count.fetch_sub(1, AK::MemoryOrder::memory_order_acq_rel);
     ASSERT(thread_cnt_before != 0);
 }
 
@@ -318,9 +302,9 @@ bool Thread::tick()
 {
     ++m_ticks;
     if (tss().cs & 3)
-        ++m_process.m_ticks_in_user;
+        ++m_process->m_ticks_in_user;
     else
-        ++m_process.m_ticks_in_kernel;
+        ++m_process->m_ticks_in_kernel;
     return --m_ticks_left;
 }
 
@@ -522,7 +506,7 @@ ShouldUnblockThread Thread::dispatch_signal(u8 signal)
             });
             [[fallthrough]];
         case DefaultSignalAction::Terminate:
-            m_process.terminate_due_to_signal(signal);
+            m_process->terminate_due_to_signal(signal);
             return ShouldUnblockThread::No;
         case DefaultSignalAction::Ignore:
             ASSERT_NOT_REACHED();
@@ -642,7 +626,7 @@ RegisterState& Thread::get_register_dump_from_stack()
 
 u32 Thread::make_userspace_stack_for_main_thread(Vector<String> arguments, Vector<String> environment, Vector<AuxiliaryValue> auxv)
 {
-    auto* region = m_process.allocate_region(VirtualAddress(), default_userspace_stack_size, "Stack (Main thread)", PROT_READ | PROT_WRITE, false);
+    auto* region = m_process->allocate_region(VirtualAddress(), default_userspace_stack_size, "Stack (Main thread)", PROT_READ | PROT_WRITE, false);
     ASSERT(region);
     region->set_stack(true);
 
@@ -714,22 +698,6 @@ Thread* Thread::clone(Process& process)
     return clone;
 }
 
-Vector<Thread*> Thread::all_threads()
-{
-    Vector<Thread*> threads;
-    InterruptDisabler disabler;
-    threads.ensure_capacity(thread_table().size());
-    for (auto* thread : thread_table())
-        threads.unchecked_append(thread);
-    return threads;
-}
-
-bool Thread::is_thread(void* ptr)
-{
-    ASSERT_INTERRUPTS_DISABLED();
-    return thread_table().contains((Thread*)ptr);
-}
-
 void Thread::set_state(State new_state)
 {
     ScopedSpinLock lock(g_scheduler_lock);
@@ -750,7 +718,7 @@ void Thread::set_state(State new_state)
     dbg() << "Set Thread " << *this << " state to " << state_string();
 #endif
 
-    if (m_process.pid() != 0) {
+    if (m_process->pid() != 0) {
         Scheduler::update_state_for_thread(*this);
     }
 
@@ -761,7 +729,7 @@ void Thread::set_state(State new_state)
     }
 }
 
-String Thread::backtrace(ProcessInspectionHandle&)
+String Thread::backtrace()
 {
     return backtrace_impl();
 }

+ 7 - 10
Kernel/Thread.h

@@ -79,15 +79,12 @@ public:
         return Processor::current().current_thread();
     }
 
-    explicit Thread(Process&);
+    explicit Thread(NonnullRefPtr<Process>);
     ~Thread();
 
     static Thread* from_tid(int);
     static void finalize_dying_threads();
 
-    static Vector<Thread*> all_threads();
-    static bool is_thread(void*);
-
     int tid() const { return m_tid; }
     int pid() const;
 
@@ -105,7 +102,7 @@ public:
     Process& process() { return m_process; }
     const Process& process() const { return m_process; }
 
-    String backtrace(ProcessInspectionHandle&);
+    String backtrace();
     Vector<FlatPtr> raw_backtrace(FlatPtr ebp, FlatPtr eip) const;
 
     const String& name() const { return m_name; }
@@ -472,13 +469,13 @@ public:
 
     void set_active(bool active)
     {
-        ASSERT(g_scheduler_lock.is_locked());
+        ASSERT(g_scheduler_lock.own_lock());
         m_is_active = active;
     }
 
     bool is_finalizable() const
     {
-        ASSERT(g_scheduler_lock.is_locked());
+        ASSERT(g_scheduler_lock.own_lock());
         return !m_is_active;
     }
 
@@ -516,7 +513,7 @@ private:
     String backtrace_impl();
     void reset_fpu_state();
 
-    Process& m_process;
+    NonnullRefPtr<Process> m_process;
     int m_tid { -1 };
     TSS32 m_tss;
     Atomic<u32> m_cpu { 0 };
@@ -633,7 +630,7 @@ template<typename Callback>
 inline IterationDecision Scheduler::for_each_runnable(Callback callback)
 {
     ASSERT_INTERRUPTS_DISABLED();
-    ASSERT(g_scheduler_lock.is_locked());
+    ASSERT(g_scheduler_lock.own_lock());
     auto& tl = g_scheduler_data->m_runnable_threads;
     for (auto it = tl.begin(); it != tl.end();) {
         auto& thread = *it;
@@ -649,7 +646,7 @@ template<typename Callback>
 inline IterationDecision Scheduler::for_each_nonrunnable(Callback callback)
 {
     ASSERT_INTERRUPTS_DISABLED();
-    ASSERT(g_scheduler_lock.is_locked());
+    ASSERT(g_scheduler_lock.own_lock());
     auto& tl = g_scheduler_data->m_nonrunnable_threads;
     for (auto it = tl.begin(); it != tl.end();) {
         auto& thread = *it;