From b7259ab38c45d5a11a3f98a4c4a91204fdde51cf Mon Sep 17 00:00:00 2001 From: Dan Klishch Date: Mon, 6 Nov 2023 23:26:04 -0500 Subject: [PATCH] LibCore: Support POSIX file actions in Core::Process::spawn --- Userland/Libraries/LibCore/Process.cpp | 172 ++++++++++++++++++------- Userland/Libraries/LibCore/Process.h | 59 +++++++++ 2 files changed, 186 insertions(+), 45 deletions(-) diff --git a/Userland/Libraries/LibCore/Process.cpp b/Userland/Libraries/LibCore/Process.cpp index 5462c66afc..f4ea72ec00 100644 --- a/Userland/Libraries/LibCore/Process.cpp +++ b/Userland/Libraries/LibCore/Process.cpp @@ -36,7 +36,6 @@ namespace Core { struct ArgvList { DeprecatedString m_path; - DeprecatedString m_working_directory; Vector m_argv; ArgvList(DeprecatedString path, size_t size) @@ -57,64 +56,108 @@ struct ArgvList { m_argv.append(nullptr); return m_argv; } - - void set_working_directory(DeprecatedString const& working_directory) - { - m_working_directory = working_directory; - } - - ErrorOr spawn(Process::KeepAsChild keep_as_child) - { -#ifdef AK_OS_SERENITY - posix_spawn_file_actions_t spawn_actions; - posix_spawn_file_actions_init(&spawn_actions); - ScopeGuard cleanup_spawn_actions = [&] { - posix_spawn_file_actions_destroy(&spawn_actions); - }; - if (!m_working_directory.is_empty()) - posix_spawn_file_actions_addchdir(&spawn_actions, m_working_directory.characters()); - - auto pid = TRY(System::posix_spawn(m_path.view(), &spawn_actions, nullptr, const_cast(get().data()), System::environment())); - if (keep_as_child == Process::KeepAsChild::No) - TRY(System::disown(pid)); -#else - auto pid = TRY(System::posix_spawn(m_path.view(), nullptr, nullptr, const_cast(get().data()), System::environment())); - // FIXME: Support keep_as_child outside Serenity. - (void)keep_as_child; -#endif - return pid; - } }; +ErrorOr Process::spawn(ProcessSpawnOptions const& options) +{ +#define CHECK(invocation) \ + if (int returned_errno = (invocation)) \ + return Error::from_errno(returned_errno); + + posix_spawn_file_actions_t spawn_actions; + CHECK(posix_spawn_file_actions_init(&spawn_actions)); + ScopeGuard cleanup_spawn_actions = [&] { + posix_spawn_file_actions_destroy(&spawn_actions); + }; + + if (options.working_directory.has_value()) { +#ifdef AK_OS_SERENITY + CHECK(posix_spawn_file_actions_addchdir(&spawn_actions, options.working_directory->characters())); +#else + // FIXME: Support ProcessSpawnOptions::working_directory n platforms that support it. + TODO(); +#endif + } + + for (auto const& file_action : options.file_actions) { + TRY(file_action.visit( + [&](FileAction::OpenFile const& action) -> ErrorOr { + CHECK(posix_spawn_file_actions_addopen( + &spawn_actions, + action.fd, + action.path.characters(), + File::open_mode_to_options(action.mode | Core::File::OpenMode::KeepOnExec), + action.permissions)); + return {}; + })); + } + +#undef CHECK + + ArgvList argv_list(options.path, options.arguments.size()); + for (auto const& argument : options.arguments) + argv_list.append(argument.characters()); + + auto pid = TRY(System::posix_spawn(options.path.view(), &spawn_actions, nullptr, const_cast(argv_list.get().data()), System::environment())); + + return Process { pid }; +} + ErrorOr Process::spawn(StringView path, ReadonlySpan arguments, DeprecatedString working_directory, KeepAsChild keep_as_child) { - ArgvList argv { path, arguments.size() }; - for (auto const& arg : arguments) - argv.append(arg.characters()); - argv.set_working_directory(working_directory); - return argv.spawn(keep_as_child); + auto process = TRY(spawn({ + .path = path, + .arguments = Vector { arguments }, + .working_directory = working_directory.is_empty() ? Optional {} : Optional { working_directory }, + })); + + if (keep_as_child == KeepAsChild::No) + TRY(process.disown()); + else { + // FIXME: This won't be needed if return value is changed to Process. + process.m_should_disown = false; + } + return process.pid(); } ErrorOr Process::spawn(StringView path, ReadonlySpan arguments, DeprecatedString working_directory, KeepAsChild keep_as_child) { Vector backing_strings; backing_strings.ensure_capacity(arguments.size()); - ArgvList argv { path, arguments.size() }; - for (auto const& arg : arguments) { - backing_strings.append(arg); - argv.append(backing_strings.last().characters()); - } - argv.set_working_directory(working_directory); - return argv.spawn(keep_as_child); + for (auto const& argument : arguments) + backing_strings.append(argument); + + auto process = TRY(spawn({ + .path = path, + .arguments = backing_strings, + .working_directory = working_directory.is_empty() ? Optional {} : Optional { working_directory }, + })); + + if (keep_as_child == KeepAsChild::No) + TRY(process.disown()); + else + process.m_should_disown = false; + return process.pid(); } ErrorOr Process::spawn(StringView path, ReadonlySpan arguments, DeprecatedString working_directory, KeepAsChild keep_as_child) { - ArgvList argv { path, arguments.size() }; - for (auto arg : arguments) - argv.append(arg); - argv.set_working_directory(working_directory); - return argv.spawn(keep_as_child); + Vector backing_strings; + backing_strings.ensure_capacity(arguments.size()); + for (auto const& argument : arguments) + backing_strings.append(argument); + + auto process = TRY(spawn({ + .path = path, + .arguments = backing_strings, + .working_directory = working_directory.is_empty() ? Optional {} : Optional { working_directory }, + })); + + if (keep_as_child == KeepAsChild::No) + TRY(process.disown()); + else + process.m_should_disown = false; + return process.pid(); } ErrorOr Process::get_name() @@ -260,4 +303,43 @@ void Process::wait_for_debugger_and_break() } } +ErrorOr Process::disown() +{ + if (m_pid != 0 && m_should_disown) { +#ifdef AK_OS_SERENITY + TRY(System::disown(m_pid)); +#else + // FIXME: Support disown outside Serenity. +#endif + m_should_disown = false; + return {}; + } else { + return Error::from_errno(EINVAL); + } +} + +ErrorOr Process::wait_for_termination() +{ + VERIFY(m_pid > 0); + + bool exited_with_code_0 = true; + int status; + if (waitpid(m_pid, &status, 0) == -1) + return Error::from_syscall("waitpid"sv, errno); + + if (WIFEXITED(status)) { + exited_with_code_0 &= WEXITSTATUS(status) == 0; + } else if (WIFSIGNALED(status)) { + exited_with_code_0 = false; + } else if (WIFSTOPPED(status)) { + // This is only possible if the child process is being traced by us. + VERIFY_NOT_REACHED(); + } else { + VERIFY_NOT_REACHED(); + } + + m_should_disown = false; + return exited_with_code_0; +} + } diff --git a/Userland/Libraries/LibCore/Process.h b/Userland/Libraries/LibCore/Process.h index f1e9ee5be4..36db835d1f 100644 --- a/Userland/Libraries/LibCore/Process.h +++ b/Userland/Libraries/LibCore/Process.h @@ -8,20 +8,62 @@ #pragma once +#include #include #include +#include namespace Core { +namespace FileAction { + +struct OpenFile { + DeprecatedString path; + File::OpenMode mode = File::OpenMode::NotOpen; + int fd = -1; + mode_t permissions = 0600; +}; + +// FIXME: Implement other file actions + +} + +struct ProcessSpawnOptions { + DeprecatedString path; + Vector const& arguments = {}; + Optional working_directory = {}; + Vector> const& file_actions = {}; +}; + class Process { + AK_MAKE_NONCOPYABLE(Process); + public: enum class KeepAsChild { Yes, No }; + Process(Process&& other) + : m_pid(exchange(other.m_pid, 0)) + , m_should_disown(exchange(other.m_should_disown, false)) + { + } + + Process& operator=(Process&& other) = delete; + + ~Process() + { + (void)disown(); + } + + static ErrorOr spawn(ProcessSpawnOptions const& options); + + // FIXME: Make the following 2 functions return Process instance or delete them. static ErrorOr spawn(StringView path, ReadonlySpan arguments, DeprecatedString working_directory = {}, KeepAsChild keep_as_child = KeepAsChild::No); static ErrorOr spawn(StringView path, ReadonlySpan arguments, DeprecatedString working_directory = {}, KeepAsChild keep_as_child = KeepAsChild::No); + + // FIXME: Remove this. char const* should not exist on this level of abstraction. static ErrorOr spawn(StringView path, ReadonlySpan arguments = {}, DeprecatedString working_directory = {}, KeepAsChild keep_as_child = KeepAsChild::No); static ErrorOr get_name(); @@ -33,6 +75,23 @@ public: static void wait_for_debugger_and_break(); static ErrorOr is_being_debugged(); + + pid_t pid() const { return m_pid; } + + ErrorOr disown(); + + // FIXME: Make it return an exit code. + ErrorOr wait_for_termination(); + +private: + Process(pid_t pid) + : m_pid(pid) + , m_should_disown(true) + { + } + + pid_t m_pid; + bool m_should_disown; }; }