From 65438d8a850cd79c343c8e92cec92f9f699f2110 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 2 Apr 2023 18:35:32 +0200 Subject: [PATCH] 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. --- Kernel/Coredump.cpp | 4 ++-- Kernel/Coredump.h | 6 +++--- Kernel/Devices/AsyncDeviceRequest.h | 2 +- Kernel/FileSystem/ProcFS/Inode.cpp | 2 +- Kernel/FileSystem/ProcFS/Inode.h | 2 +- Kernel/Process.cpp | 24 ++++++++++++------------ Kernel/Process.h | 14 +++++++------- Kernel/Syscalls/waitid.cpp | 4 ++-- Kernel/Thread.cpp | 6 +++--- Kernel/Thread.h | 14 +++++++------- Kernel/ThreadBlockers.cpp | 6 +++--- 11 files changed, 42 insertions(+), 42 deletions(-) diff --git a/Kernel/Coredump.cpp b/Kernel/Coredump.cpp index ccc0131ea4a..ce23c9ddc35 100644 --- a/Kernel/Coredump.cpp +++ b/Kernel/Coredump.cpp @@ -52,7 +52,7 @@ bool Coredump::FlatRegionData::is_consistent_with_region(Memory::Region const& r return true; } -ErrorOr> Coredump::try_create(NonnullLockRefPtr process, StringView output_path) +ErrorOr> Coredump::try_create(NonnullRefPtr 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> Coredump::try_create(NonnullLockRefPtr return adopt_nonnull_own_or_enomem(new (nothrow) Coredump(move(process), move(description), move(regions))); } -Coredump::Coredump(NonnullLockRefPtr process, NonnullRefPtr description, Vector regions) +Coredump::Coredump(NonnullRefPtr process, NonnullRefPtr description, Vector regions) : m_process(move(process)) , m_description(move(description)) , m_regions(move(regions)) diff --git a/Kernel/Coredump.h b/Kernel/Coredump.h index 29b7d5ef7f0..79873a1cc03 100644 --- a/Kernel/Coredump.h +++ b/Kernel/Coredump.h @@ -18,7 +18,7 @@ namespace Kernel { class Coredump { public: - static ErrorOr> try_create(NonnullLockRefPtr, StringView output_path); + static ErrorOr> try_create(NonnullRefPtr, StringView output_path); static SpinlockProtected, LockRank::None>& directory_path(); ~Coredump() = default; @@ -65,7 +65,7 @@ private: VirtualAddress m_vaddr; }; - Coredump(NonnullLockRefPtr, NonnullRefPtr, Vector); + Coredump(NonnullRefPtr, NonnullRefPtr, Vector); static ErrorOr> try_create_target_file(Process const&, StringView output_path); ErrorOr write_elf_header(); @@ -79,7 +79,7 @@ private: ErrorOr create_notes_regions_data(auto&) const; ErrorOr create_notes_metadata_data(auto&) const; - NonnullLockRefPtr m_process; + NonnullRefPtr const m_process; NonnullRefPtr m_description; size_t m_num_program_headers { 0 }; Vector m_regions; diff --git a/Kernel/Devices/AsyncDeviceRequest.h b/Kernel/Devices/AsyncDeviceRequest.h index 449b2f9d84d..350ded3d11a 100644 --- a/Kernel/Devices/AsyncDeviceRequest.h +++ b/Kernel/Devices/AsyncDeviceRequest.h @@ -149,7 +149,7 @@ private: AsyncDeviceSubRequestList m_sub_requests_pending; AsyncDeviceSubRequestList m_sub_requests_complete; WaitQueue m_queue; - NonnullLockRefPtr m_process; + NonnullRefPtr const m_process; void* m_private { nullptr }; mutable Spinlock m_lock {}; }; diff --git a/Kernel/FileSystem/ProcFS/Inode.cpp b/Kernel/FileSystem/ProcFS/Inode.cpp index 30287c6abc0..7140ea7e4d1 100644 --- a/Kernel/FileSystem/ProcFS/Inode.cpp +++ b/Kernel/FileSystem/ProcFS/Inode.cpp @@ -288,7 +288,7 @@ static ErrorOr build_from_cached_data(KBufferBuilder& builder, ProcFSInode return {}; } -ErrorOr ProcFSInode::try_fetch_process_property_data(NonnullLockRefPtr process, KBufferBuilder& builder) const +ErrorOr ProcFSInode::try_fetch_process_property_data(NonnullRefPtr process, KBufferBuilder& builder) const { VERIFY(m_type == Type::ProcessProperty); if (m_subdirectory == process_fd_subdirectory_root_entry.subdirectory) { diff --git a/Kernel/FileSystem/ProcFS/Inode.h b/Kernel/FileSystem/ProcFS/Inode.h index 8a2b62b7695..d060293dd8a 100644 --- a/Kernel/FileSystem/ProcFS/Inode.h +++ b/Kernel/FileSystem/ProcFS/Inode.h @@ -69,7 +69,7 @@ private: virtual ErrorOr> lookup(StringView name) override final; ErrorOr refresh_process_property_data(OpenFileDescription& description); - ErrorOr try_fetch_process_property_data(NonnullLockRefPtr, KBufferBuilder& builder) const; + ErrorOr try_fetch_process_property_data(NonnullRefPtr, KBufferBuilder& builder) const; Type m_type; Optional const m_associated_pid {}; diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 2783dce4aa7..eeeb052e382 100644 --- a/Kernel/Process.cpp +++ b/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 new_process = process; + NonnullRefPtr const new_process = process; all_instances().with([&](auto& list) { list.prepend(process); }); } -ErrorOr> Process::try_create_user_process(LockRefPtr& first_thread, StringView path, UserID uid, GroupID gid, Vector> arguments, Vector> environment, TTY* tty) +ErrorOr> Process::try_create_user_process(LockRefPtr& first_thread, StringView path, UserID uid, GroupID gid, Vector> arguments, Vector> environment, TTY* tty) { auto parts = path.split_view('/'); if (arguments.is_empty()) { @@ -257,7 +257,7 @@ ErrorOr> Process::try_create_user_process(LockRefPtr< return process; } -LockRefPtr Process::create_kernel_process(LockRefPtr& first_thread, NonnullOwnPtr name, void (*entry)(void*), void* entry_data, u32 affinity, RegisterProcess do_register) +RefPtr Process::create_kernel_process(LockRefPtr& first_thread, NonnullOwnPtr 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> Process::try_create(LockRefPtr& first_thread, NonnullOwnPtr name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, RefPtr current_directory, RefPtr executable, TTY* tty, Process* fork_parent) +ErrorOr> Process::try_create(LockRefPtr& first_thread, NonnullOwnPtr name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, RefPtr current_directory, RefPtr executable, TTY* tty, Process* fork_parent) { OwnPtr new_address_space; if (fork_parent) { @@ -303,7 +303,7 @@ ErrorOr> Process::try_create(LockRefPtr& 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 regs, bool out_of VERIFY_NOT_REACHED(); } -LockRefPtr Process::from_pid_in_same_jail(ProcessID pid) +RefPtr Process::from_pid_in_same_jail(ProcessID pid) { - return Process::current().m_jail_process_list.with([&](auto const& list_ptr) -> LockRefPtr { + return Process::current().m_jail_process_list.with([&](auto const& list_ptr) -> RefPtr { if (list_ptr) { - return list_ptr->attached_processes().with([&](auto const& list) -> LockRefPtr { + return list_ptr->attached_processes().with([&](auto const& list) -> RefPtr { for (auto& process : list) { if (process.pid() == pid) { return process; @@ -504,7 +504,7 @@ LockRefPtr Process::from_pid_in_same_jail(ProcessID pid) return {}; }); } - return all_instances().with([&](auto const& list) -> LockRefPtr { + return all_instances().with([&](auto const& list) -> RefPtr { for (auto& process : list) { if (process.pid() == pid) { return process; @@ -515,9 +515,9 @@ LockRefPtr Process::from_pid_in_same_jail(ProcessID pid) }); } -LockRefPtr Process::from_pid_ignoring_jails(ProcessID pid) +RefPtr Process::from_pid_ignoring_jails(ProcessID pid) { - return all_instances().with([&](auto const& list) -> LockRefPtr { + return all_instances().with([&](auto const& list) -> RefPtr { 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 waiter_process; + RefPtr waiter_process; if (auto* my_tracer = tracer()) waiter_process = Process::from_pid_ignoring_jails(my_tracer->tracer_pid()); else diff --git a/Kernel/Process.h b/Kernel/Process.h index 12027a4c978..e1b5220fcb9 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -186,14 +186,14 @@ public: }; template - static LockRefPtr create_kernel_process(LockRefPtr& first_thread, NonnullOwnPtr name, EntryFunction entry, u32 affinity = THREAD_AFFINITY_DEFAULT, RegisterProcess do_register = RegisterProcess::Yes) + static RefPtr create_kernel_process(LockRefPtr& first_thread, NonnullOwnPtr 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, entry_func, affinity, do_register); } - static LockRefPtr create_kernel_process(LockRefPtr& first_thread, NonnullOwnPtr name, void (*entry)(void*), void* entry_data = nullptr, u32 affinity = THREAD_AFFINITY_DEFAULT, RegisterProcess do_register = RegisterProcess::Yes); - static ErrorOr> try_create_user_process(LockRefPtr& first_thread, StringView path, UserID, GroupID, Vector> arguments, Vector> environment, TTY*); + static RefPtr create_kernel_process(LockRefPtr& first_thread, NonnullOwnPtr name, void (*entry)(void*), void* entry_data = nullptr, u32 affinity = THREAD_AFFINITY_DEFAULT, RegisterProcess do_register = RegisterProcess::Yes); + static ErrorOr> try_create_user_process(LockRefPtr& first_thread, StringView path, UserID, GroupID, Vector> arguments, Vector> 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 from_pid_in_same_jail(ProcessID); - static LockRefPtr from_pid_ignoring_jails(ProcessID); + static RefPtr from_pid_in_same_jail(ProcessID); + static RefPtr from_pid_ignoring_jails(ProcessID); static SessionID get_sid_from_pgid(ProcessGroupID pgid); SpinlockProtected, LockRank::None> const& name() const; @@ -594,7 +594,7 @@ private: bool remove_thread(Thread&); Process(NonnullOwnPtr name, NonnullRefPtr, ProcessID ppid, bool is_kernel_process, RefPtr current_directory, RefPtr executable, TTY* tty, UnveilNode unveil_tree, UnveilNode exec_unveil_tree); - static ErrorOr> try_create(LockRefPtr& first_thread, NonnullOwnPtr name, UserID, GroupID, ProcessID ppid, bool is_kernel_process, RefPtr current_directory = nullptr, RefPtr executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr); + static ErrorOr> try_create(LockRefPtr& first_thread, NonnullOwnPtr name, UserID, GroupID, ProcessID ppid, bool is_kernel_process, RefPtr current_directory = nullptr, RefPtr executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr); ErrorOr attach_resources(NonnullOwnPtr&&, LockRefPtr& first_thread, Process* fork_parent); static ProcessID allocate_pid(); @@ -617,7 +617,7 @@ private: ErrorOr do_killall(int signal); ErrorOr do_killself(int signal); - ErrorOr do_waitid(Variant, NonnullLockRefPtr> waitee, int options); + ErrorOr do_waitid(Variant, NonnullLockRefPtr> waitee, int options); static ErrorOr> get_syscall_path_argument(Userspace user_path, size_t path_length); static ErrorOr> get_syscall_path_argument(Syscall::StringArgument const&); diff --git a/Kernel/Syscalls/waitid.cpp b/Kernel/Syscalls/waitid.cpp index 83983766581..8e138251892 100644 --- a/Kernel/Syscalls/waitid.cpp +++ b/Kernel/Syscalls/waitid.cpp @@ -10,7 +10,7 @@ namespace Kernel { -ErrorOr Process::do_waitid(Variant, NonnullLockRefPtr> waitee, int options) +ErrorOr Process::do_waitid(Variant, NonnullLockRefPtr> waitee, int options) { ErrorOr result = siginfo_t {}; if (Thread::current()->block({}, options, move(waitee), result).was_interrupted()) @@ -25,7 +25,7 @@ ErrorOr Process::sys$waitid(Userspace TRY(require_promise(Pledge::proc)); auto params = TRY(copy_typed_from_user(user_params)); - Variant, NonnullLockRefPtr> waitee; + Variant, NonnullLockRefPtr> waitee; switch (params.idtype) { case P_ALL: break; diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index 36f31416732..40b9a3843c8 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -39,7 +39,7 @@ SpinlockProtected& Thread::all_instances() return *s_list; } -ErrorOr> Thread::try_create(NonnullLockRefPtr process) +ErrorOr> Thread::try_create(NonnullRefPtr 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> Thread::try_create(NonnullLockRefPtr 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, NonnullOwnPtr kernel_stack_region, NonnullLockRefPtr block_timer, NonnullOwnPtr name) +Thread::Thread(NonnullRefPtr process, NonnullOwnPtr kernel_stack_region, NonnullLockRefPtr block_timer, NonnullOwnPtr 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 = thread->process(); + RefPtr 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(); diff --git a/Kernel/Thread.h b/Kernel/Thread.h index 2a18283c575..6738e8bb83e 100644 --- a/Kernel/Thread.h +++ b/Kernel/Thread.h @@ -67,7 +67,7 @@ public: return Processor::current_thread(); } - static ErrorOr> try_create(NonnullLockRefPtr); + static ErrorOr> try_create(NonnullRefPtr); ~Thread(); static LockRefPtr from_tid(ThreadID); @@ -644,7 +644,7 @@ public: Disowned }; - WaitBlocker(int wait_options, Variant, NonnullLockRefPtr> waitee, ErrorOr& result); + WaitBlocker(int wait_options, Variant, NonnullLockRefPtr> waitee, ErrorOr& 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& m_result; - Variant, NonnullLockRefPtr> m_waitee; + Variant, NonnullLockRefPtr> const m_waitee; bool m_did_unblock { false }; bool m_got_sigchild { false }; }; @@ -684,12 +684,12 @@ public: private: struct ProcessBlockInfo { - NonnullLockRefPtr process; + NonnullRefPtr const process; WaitBlocker::UnblockFlags flags; u8 signal; bool was_waited { false }; - explicit ProcessBlockInfo(NonnullLockRefPtr&&, WaitBlocker::UnblockFlags, u8); + explicit ProcessBlockInfo(NonnullRefPtr&&, WaitBlocker::UnblockFlags, u8); ~ProcessBlockInfo(); }; @@ -1083,7 +1083,7 @@ public: #endif private: - Thread(NonnullLockRefPtr, NonnullOwnPtr, NonnullLockRefPtr, NonnullOwnPtr); + Thread(NonnullRefPtr, NonnullOwnPtr, NonnullLockRefPtr, NonnullOwnPtr); BlockResult block_impl(BlockTimeout const&, Blocker&); @@ -1154,7 +1154,7 @@ private: mutable RecursiveSpinlock m_lock {}; mutable RecursiveSpinlock m_block_lock {}; - NonnullLockRefPtr m_process; + NonnullRefPtr const m_process; ThreadID m_tid { -1 }; ThreadRegisters m_regs {}; DebugRegisterState m_debug_register_state {}; diff --git a/Kernel/ThreadBlockers.cpp b/Kernel/ThreadBlockers.cpp index 9181373ad0a..c88df1539ee 100644 --- a/Kernel/ThreadBlockers.cpp +++ b/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, WaitBlocker::UnblockFlags flags, u8 signal) +Thread::WaitBlockerSet::ProcessBlockInfo::ProcessBlockInfo(NonnullRefPtr&& 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, NonnullLockRefPtr> waitee, ErrorOr& result) +Thread::WaitBlocker::WaitBlocker(int wait_options, Variant, NonnullLockRefPtr> waitee, ErrorOr& 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 const& waitee_process) { + [&](NonnullRefPtr const& waitee_process) { return &process != waitee_process; }, [&](NonnullLockRefPtr const& waitee_process_group) {