Parcourir la source

Kernel: Stop using *LockRefPtr for Process pointers

The only persistent one of these was Thread::m_process and that never
changes after initialization. Make it const to enforce this and switch
everything over to RefPtr & NonnullRefPtr.
Andreas Kling il y a 2 ans
Parent
commit
65438d8a85

+ 2 - 2
Kernel/Coredump.cpp

@@ -52,7 +52,7 @@ bool Coredump::FlatRegionData::is_consistent_with_region(Memory::Region const& r
     return true;
 }
 
-ErrorOr<NonnullOwnPtr<Coredump>> Coredump::try_create(NonnullLockRefPtr<Process> process, StringView output_path)
+ErrorOr<NonnullOwnPtr<Coredump>> Coredump::try_create(NonnullRefPtr<Process> process, StringView output_path)
 {
     if (!process->is_dumpable()) {
         dbgln("Refusing to generate coredump for non-dumpable process {}", process->pid().value());
@@ -74,7 +74,7 @@ ErrorOr<NonnullOwnPtr<Coredump>> Coredump::try_create(NonnullLockRefPtr<Process>
     return adopt_nonnull_own_or_enomem(new (nothrow) Coredump(move(process), move(description), move(regions)));
 }
 
-Coredump::Coredump(NonnullLockRefPtr<Process> process, NonnullRefPtr<OpenFileDescription> description, Vector<FlatRegionData> regions)
+Coredump::Coredump(NonnullRefPtr<Process> process, NonnullRefPtr<OpenFileDescription> description, Vector<FlatRegionData> regions)
     : m_process(move(process))
     , m_description(move(description))
     , m_regions(move(regions))

+ 3 - 3
Kernel/Coredump.h

@@ -18,7 +18,7 @@ namespace Kernel {
 
 class Coredump {
 public:
-    static ErrorOr<NonnullOwnPtr<Coredump>> try_create(NonnullLockRefPtr<Process>, StringView output_path);
+    static ErrorOr<NonnullOwnPtr<Coredump>> try_create(NonnullRefPtr<Process>, StringView output_path);
     static SpinlockProtected<OwnPtr<KString>, LockRank::None>& directory_path();
 
     ~Coredump() = default;
@@ -65,7 +65,7 @@ private:
         VirtualAddress m_vaddr;
     };
 
-    Coredump(NonnullLockRefPtr<Process>, NonnullRefPtr<OpenFileDescription>, Vector<FlatRegionData>);
+    Coredump(NonnullRefPtr<Process>, NonnullRefPtr<OpenFileDescription>, Vector<FlatRegionData>);
     static ErrorOr<NonnullRefPtr<OpenFileDescription>> try_create_target_file(Process const&, StringView output_path);
 
     ErrorOr<void> write_elf_header();
@@ -79,7 +79,7 @@ private:
     ErrorOr<void> create_notes_regions_data(auto&) const;
     ErrorOr<void> create_notes_metadata_data(auto&) const;
 
-    NonnullLockRefPtr<Process> m_process;
+    NonnullRefPtr<Process> const m_process;
     NonnullRefPtr<OpenFileDescription> m_description;
     size_t m_num_program_headers { 0 };
     Vector<FlatRegionData> m_regions;

+ 1 - 1
Kernel/Devices/AsyncDeviceRequest.h

@@ -149,7 +149,7 @@ private:
     AsyncDeviceSubRequestList m_sub_requests_pending;
     AsyncDeviceSubRequestList m_sub_requests_complete;
     WaitQueue m_queue;
-    NonnullLockRefPtr<Process> m_process;
+    NonnullRefPtr<Process> const m_process;
     void* m_private { nullptr };
     mutable Spinlock<LockRank::None> m_lock {};
 };

+ 1 - 1
Kernel/FileSystem/ProcFS/Inode.cpp

@@ -288,7 +288,7 @@ static ErrorOr<void> build_from_cached_data(KBufferBuilder& builder, ProcFSInode
     return {};
 }
 
-ErrorOr<void> ProcFSInode::try_fetch_process_property_data(NonnullLockRefPtr<Process> process, KBufferBuilder& builder) const
+ErrorOr<void> ProcFSInode::try_fetch_process_property_data(NonnullRefPtr<Process> process, KBufferBuilder& builder) const
 {
     VERIFY(m_type == Type::ProcessProperty);
     if (m_subdirectory == process_fd_subdirectory_root_entry.subdirectory) {

+ 1 - 1
Kernel/FileSystem/ProcFS/Inode.h

@@ -69,7 +69,7 @@ private:
     virtual ErrorOr<NonnullRefPtr<Inode>> lookup(StringView name) override final;
 
     ErrorOr<void> refresh_process_property_data(OpenFileDescription& description);
-    ErrorOr<void> try_fetch_process_property_data(NonnullLockRefPtr<Process>, KBufferBuilder& builder) const;
+    ErrorOr<void> try_fetch_process_property_data(NonnullRefPtr<Process>, KBufferBuilder& builder) const;
 
     Type m_type;
     Optional<ProcessID> const m_associated_pid {};

+ 12 - 12
Kernel/Process.cpp

@@ -202,13 +202,13 @@ void Process::kill_all_threads()
 void Process::register_new(Process& process)
 {
     // Note: this is essentially the same like process->ref()
-    LockRefPtr<Process> new_process = process;
+    NonnullRefPtr<Process> const new_process = process;
     all_instances().with([&](auto& list) {
         list.prepend(process);
     });
 }
 
-ErrorOr<NonnullLockRefPtr<Process>> Process::try_create_user_process(LockRefPtr<Thread>& first_thread, StringView path, UserID uid, GroupID gid, Vector<NonnullOwnPtr<KString>> arguments, Vector<NonnullOwnPtr<KString>> environment, TTY* tty)
+ErrorOr<NonnullRefPtr<Process>> Process::try_create_user_process(LockRefPtr<Thread>& first_thread, StringView path, UserID uid, GroupID gid, Vector<NonnullOwnPtr<KString>> arguments, Vector<NonnullOwnPtr<KString>> environment, TTY* tty)
 {
     auto parts = path.split_view('/');
     if (arguments.is_empty()) {
@@ -257,7 +257,7 @@ ErrorOr<NonnullLockRefPtr<Process>> Process::try_create_user_process(LockRefPtr<
     return process;
 }
 
-LockRefPtr<Process> Process::create_kernel_process(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, void (*entry)(void*), void* entry_data, u32 affinity, RegisterProcess do_register)
+RefPtr<Process> Process::create_kernel_process(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, void (*entry)(void*), void* entry_data, u32 affinity, RegisterProcess do_register)
 {
     auto process_or_error = Process::try_create(first_thread, move(name), UserID(0), GroupID(0), ProcessID(0), true);
     if (process_or_error.is_error())
@@ -289,7 +289,7 @@ void Process::unprotect_data()
     });
 }
 
-ErrorOr<NonnullLockRefPtr<Process>> Process::try_create(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory, RefPtr<Custody> executable, TTY* tty, Process* fork_parent)
+ErrorOr<NonnullRefPtr<Process>> Process::try_create(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory, RefPtr<Custody> executable, TTY* tty, Process* fork_parent)
 {
     OwnPtr<Memory::AddressSpace> new_address_space;
     if (fork_parent) {
@@ -303,7 +303,7 @@ ErrorOr<NonnullLockRefPtr<Process>> Process::try_create(LockRefPtr<Thread>& firs
     auto unveil_tree = UnveilNode { TRY(KString::try_create("/"sv)), UnveilMetadata(TRY(KString::try_create("/"sv))) };
     auto exec_unveil_tree = UnveilNode { TRY(KString::try_create("/"sv)), UnveilMetadata(TRY(KString::try_create("/"sv))) };
     auto credentials = TRY(Credentials::create(uid, gid, uid, gid, uid, gid, {}, fork_parent ? fork_parent->sid() : 0, fork_parent ? fork_parent->pgid() : 0));
-    auto process = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) Process(move(name), move(credentials), ppid, is_kernel_process, move(current_directory), move(executable), tty, move(unveil_tree), move(exec_unveil_tree))));
+    auto process = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Process(move(name), move(credentials), ppid, is_kernel_process, move(current_directory), move(executable), tty, move(unveil_tree), move(exec_unveil_tree))));
     TRY(process->attach_resources(new_address_space.release_nonnull(), first_thread, fork_parent));
     return process;
 }
@@ -491,11 +491,11 @@ void Process::crash(int signal, Optional<RegisterState const&> regs, bool out_of
     VERIFY_NOT_REACHED();
 }
 
-LockRefPtr<Process> Process::from_pid_in_same_jail(ProcessID pid)
+RefPtr<Process> Process::from_pid_in_same_jail(ProcessID pid)
 {
-    return Process::current().m_jail_process_list.with([&](auto const& list_ptr) -> LockRefPtr<Process> {
+    return Process::current().m_jail_process_list.with([&](auto const& list_ptr) -> RefPtr<Process> {
         if (list_ptr) {
-            return list_ptr->attached_processes().with([&](auto const& list) -> LockRefPtr<Process> {
+            return list_ptr->attached_processes().with([&](auto const& list) -> RefPtr<Process> {
                 for (auto& process : list) {
                     if (process.pid() == pid) {
                         return process;
@@ -504,7 +504,7 @@ LockRefPtr<Process> Process::from_pid_in_same_jail(ProcessID pid)
                 return {};
             });
         }
-        return all_instances().with([&](auto const& list) -> LockRefPtr<Process> {
+        return all_instances().with([&](auto const& list) -> RefPtr<Process> {
             for (auto& process : list) {
                 if (process.pid() == pid) {
                     return process;
@@ -515,9 +515,9 @@ LockRefPtr<Process> Process::from_pid_in_same_jail(ProcessID pid)
     });
 }
 
-LockRefPtr<Process> Process::from_pid_ignoring_jails(ProcessID pid)
+RefPtr<Process> Process::from_pid_ignoring_jails(ProcessID pid)
 {
-    return all_instances().with([&](auto const& list) -> LockRefPtr<Process> {
+    return all_instances().with([&](auto const& list) -> RefPtr<Process> {
         for (auto const& process : list) {
             if (process.pid() == pid)
                 return &process;
@@ -812,7 +812,7 @@ void Process::disowned_by_waiter(Process& process)
 
 void Process::unblock_waiters(Thread::WaitBlocker::UnblockFlags flags, u8 signal)
 {
-    LockRefPtr<Process> waiter_process;
+    RefPtr<Process> waiter_process;
     if (auto* my_tracer = tracer())
         waiter_process = Process::from_pid_ignoring_jails(my_tracer->tracer_pid());
     else

+ 7 - 7
Kernel/Process.h

@@ -186,14 +186,14 @@ public:
     };
 
     template<typename EntryFunction>
-    static LockRefPtr<Process> create_kernel_process(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, EntryFunction entry, u32 affinity = THREAD_AFFINITY_DEFAULT, RegisterProcess do_register = RegisterProcess::Yes)
+    static RefPtr<Process> create_kernel_process(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, EntryFunction entry, u32 affinity = THREAD_AFFINITY_DEFAULT, RegisterProcess do_register = RegisterProcess::Yes)
     {
         auto* entry_func = new EntryFunction(move(entry));
         return create_kernel_process(first_thread, move(name), &Process::kernel_process_trampoline<EntryFunction>, entry_func, affinity, do_register);
     }
 
-    static LockRefPtr<Process> create_kernel_process(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, void (*entry)(void*), void* entry_data = nullptr, u32 affinity = THREAD_AFFINITY_DEFAULT, RegisterProcess do_register = RegisterProcess::Yes);
-    static ErrorOr<NonnullLockRefPtr<Process>> try_create_user_process(LockRefPtr<Thread>& first_thread, StringView path, UserID, GroupID, Vector<NonnullOwnPtr<KString>> arguments, Vector<NonnullOwnPtr<KString>> environment, TTY*);
+    static RefPtr<Process> create_kernel_process(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, void (*entry)(void*), void* entry_data = nullptr, u32 affinity = THREAD_AFFINITY_DEFAULT, RegisterProcess do_register = RegisterProcess::Yes);
+    static ErrorOr<NonnullRefPtr<Process>> try_create_user_process(LockRefPtr<Thread>& first_thread, StringView path, UserID, GroupID, Vector<NonnullOwnPtr<KString>> arguments, Vector<NonnullOwnPtr<KString>> environment, TTY*);
     static void register_new(Process&);
 
     ~Process();
@@ -215,8 +215,8 @@ public:
     bool is_kernel_process() const { return m_is_kernel_process; }
     bool is_user_process() const { return !m_is_kernel_process; }
 
-    static LockRefPtr<Process> from_pid_in_same_jail(ProcessID);
-    static LockRefPtr<Process> from_pid_ignoring_jails(ProcessID);
+    static RefPtr<Process> from_pid_in_same_jail(ProcessID);
+    static RefPtr<Process> from_pid_ignoring_jails(ProcessID);
     static SessionID get_sid_from_pgid(ProcessGroupID pgid);
 
     SpinlockProtected<NonnullOwnPtr<KString>, LockRank::None> const& name() const;
@@ -594,7 +594,7 @@ private:
     bool remove_thread(Thread&);
 
     Process(NonnullOwnPtr<KString> name, NonnullRefPtr<Credentials>, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory, RefPtr<Custody> executable, TTY* tty, UnveilNode unveil_tree, UnveilNode exec_unveil_tree);
-    static ErrorOr<NonnullLockRefPtr<Process>> try_create(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, UserID, GroupID, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory = nullptr, RefPtr<Custody> executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr);
+    static ErrorOr<NonnullRefPtr<Process>> try_create(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, UserID, GroupID, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory = nullptr, RefPtr<Custody> executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr);
     ErrorOr<void> attach_resources(NonnullOwnPtr<Memory::AddressSpace>&&, LockRefPtr<Thread>& first_thread, Process* fork_parent);
     static ProcessID allocate_pid();
 
@@ -617,7 +617,7 @@ private:
     ErrorOr<void> do_killall(int signal);
     ErrorOr<void> do_killself(int signal);
 
-    ErrorOr<siginfo_t> do_waitid(Variant<Empty, NonnullLockRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, int options);
+    ErrorOr<siginfo_t> do_waitid(Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, int options);
 
     static ErrorOr<NonnullOwnPtr<KString>> get_syscall_path_argument(Userspace<char const*> user_path, size_t path_length);
     static ErrorOr<NonnullOwnPtr<KString>> get_syscall_path_argument(Syscall::StringArgument const&);

+ 2 - 2
Kernel/Syscalls/waitid.cpp

@@ -10,7 +10,7 @@
 
 namespace Kernel {
 
-ErrorOr<siginfo_t> Process::do_waitid(Variant<Empty, NonnullLockRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, int options)
+ErrorOr<siginfo_t> Process::do_waitid(Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, int options)
 {
     ErrorOr<siginfo_t> result = siginfo_t {};
     if (Thread::current()->block<Thread::WaitBlocker>({}, options, move(waitee), result).was_interrupted())
@@ -25,7 +25,7 @@ ErrorOr<FlatPtr> Process::sys$waitid(Userspace<Syscall::SC_waitid_params const*>
     TRY(require_promise(Pledge::proc));
     auto params = TRY(copy_typed_from_user(user_params));
 
-    Variant<Empty, NonnullLockRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee;
+    Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee;
     switch (params.idtype) {
     case P_ALL:
         break;

+ 3 - 3
Kernel/Thread.cpp

@@ -39,7 +39,7 @@ SpinlockProtected<Thread::GlobalList, LockRank::None>& Thread::all_instances()
     return *s_list;
 }
 
-ErrorOr<NonnullLockRefPtr<Thread>> Thread::try_create(NonnullLockRefPtr<Process> process)
+ErrorOr<NonnullLockRefPtr<Thread>> Thread::try_create(NonnullRefPtr<Process> process)
 {
     auto kernel_stack_region = TRY(MM.allocate_kernel_region(default_kernel_stack_size, {}, Memory::Region::Access::ReadWrite, AllocationStrategy::AllocateNow));
     kernel_stack_region->set_stack(true);
@@ -50,7 +50,7 @@ ErrorOr<NonnullLockRefPtr<Thread>> Thread::try_create(NonnullLockRefPtr<Process>
     return adopt_nonnull_lock_ref_or_enomem(new (nothrow) Thread(move(process), move(kernel_stack_region), move(block_timer), move(name)));
 }
 
-Thread::Thread(NonnullLockRefPtr<Process> process, NonnullOwnPtr<Memory::Region> kernel_stack_region, NonnullLockRefPtr<Timer> block_timer, NonnullOwnPtr<KString> name)
+Thread::Thread(NonnullRefPtr<Process> process, NonnullOwnPtr<Memory::Region> kernel_stack_region, NonnullLockRefPtr<Timer> block_timer, NonnullOwnPtr<KString> name)
     : m_process(move(process))
     , m_kernel_stack_region(move(kernel_stack_region))
     , m_name(move(name))
@@ -596,7 +596,7 @@ void Thread::finalize_dying_threads()
         });
     }
     for (auto* thread : dying_threads) {
-        LockRefPtr<Process> process = thread->process();
+        RefPtr<Process> const process = thread->process();
         dbgln_if(PROCESS_DEBUG, "Before finalization, {} has {} refs and its process has {}",
             *thread, thread->ref_count(), thread->process().ref_count());
         thread->finalize();

+ 7 - 7
Kernel/Thread.h

@@ -67,7 +67,7 @@ public:
         return Processor::current_thread();
     }
 
-    static ErrorOr<NonnullLockRefPtr<Thread>> try_create(NonnullLockRefPtr<Process>);
+    static ErrorOr<NonnullLockRefPtr<Thread>> try_create(NonnullRefPtr<Process>);
     ~Thread();
 
     static LockRefPtr<Thread> from_tid(ThreadID);
@@ -644,7 +644,7 @@ public:
             Disowned
         };
 
-        WaitBlocker(int wait_options, Variant<Empty, NonnullLockRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, ErrorOr<siginfo_t>& result);
+        WaitBlocker(int wait_options, Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, ErrorOr<siginfo_t>& result);
         virtual StringView state_string() const override { return "Waiting"sv; }
         virtual Type blocker_type() const override { return Type::Wait; }
         virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override;
@@ -660,7 +660,7 @@ public:
 
         int const m_wait_options;
         ErrorOr<siginfo_t>& m_result;
-        Variant<Empty, NonnullLockRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> m_waitee;
+        Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> const m_waitee;
         bool m_did_unblock { false };
         bool m_got_sigchild { false };
     };
@@ -684,12 +684,12 @@ public:
 
     private:
         struct ProcessBlockInfo {
-            NonnullLockRefPtr<Process> process;
+            NonnullRefPtr<Process> const process;
             WaitBlocker::UnblockFlags flags;
             u8 signal;
             bool was_waited { false };
 
-            explicit ProcessBlockInfo(NonnullLockRefPtr<Process>&&, WaitBlocker::UnblockFlags, u8);
+            explicit ProcessBlockInfo(NonnullRefPtr<Process>&&, WaitBlocker::UnblockFlags, u8);
             ~ProcessBlockInfo();
         };
 
@@ -1083,7 +1083,7 @@ public:
 #endif
 
 private:
-    Thread(NonnullLockRefPtr<Process>, NonnullOwnPtr<Memory::Region>, NonnullLockRefPtr<Timer>, NonnullOwnPtr<KString>);
+    Thread(NonnullRefPtr<Process>, NonnullOwnPtr<Memory::Region>, NonnullLockRefPtr<Timer>, NonnullOwnPtr<KString>);
 
     BlockResult block_impl(BlockTimeout const&, Blocker&);
 
@@ -1154,7 +1154,7 @@ private:
 
     mutable RecursiveSpinlock<LockRank::Thread> m_lock {};
     mutable RecursiveSpinlock<LockRank::None> m_block_lock {};
-    NonnullLockRefPtr<Process> m_process;
+    NonnullRefPtr<Process> const m_process;
     ThreadID m_tid { -1 };
     ThreadRegisters m_regs {};
     DebugRegisterState m_debug_register_state {};

+ 3 - 3
Kernel/ThreadBlockers.cpp

@@ -510,7 +510,7 @@ bool Thread::SignalBlocker::check_pending_signals(bool from_add_blocker)
     return true;
 }
 
-Thread::WaitBlockerSet::ProcessBlockInfo::ProcessBlockInfo(NonnullLockRefPtr<Process>&& process, WaitBlocker::UnblockFlags flags, u8 signal)
+Thread::WaitBlockerSet::ProcessBlockInfo::ProcessBlockInfo(NonnullRefPtr<Process>&& process, WaitBlocker::UnblockFlags flags, u8 signal)
     : process(move(process))
     , flags(flags)
     , signal(signal)
@@ -663,7 +663,7 @@ void Thread::WaitBlockerSet::finalize()
     }
 }
 
-Thread::WaitBlocker::WaitBlocker(int wait_options, Variant<Empty, NonnullLockRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, ErrorOr<siginfo_t>& result)
+Thread::WaitBlocker::WaitBlocker(int wait_options, Variant<Empty, NonnullRefPtr<Process>, NonnullLockRefPtr<ProcessGroup>> waitee, ErrorOr<siginfo_t>& result)
     : m_wait_options(wait_options)
     , m_result(result)
     , m_waitee(move(waitee))
@@ -731,7 +731,7 @@ bool Thread::WaitBlocker::unblock(Process& process, UnblockFlags flags, u8 signa
     VERIFY(flags != UnblockFlags::Terminated || signal == 0); // signal argument should be ignored for Terminated
 
     bool do_not_unblock = m_waitee.visit(
-        [&](NonnullLockRefPtr<Process> const& waitee_process) {
+        [&](NonnullRefPtr<Process> const& waitee_process) {
             return &process != waitee_process;
         },
         [&](NonnullLockRefPtr<ProcessGroup> const& waitee_process_group) {