From 75b3cf266d3829e8a150d1101974b099d940a78f Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 14 Apr 2022 16:39:34 +0200 Subject: [PATCH] Add Process functions for detached spawning --- src/common/logging/common.h | 3 +- src/common/process.cpp | 134 +++++++++++++++++++++++++++++--- src/common/process.h | 25 +++++- src/wine-host/bridges/group.cpp | 2 +- src/wine-host/xdnd-proxy.h | 4 +- 5 files changed, 152 insertions(+), 16 deletions(-) diff --git a/src/common/logging/common.h b/src/common/logging/common.h index 70f700cb..a4afe8cd 100644 --- a/src/common/logging/common.h +++ b/src/common/logging/common.h @@ -66,11 +66,12 @@ class patched_async_pipe { inline patched_async_pipe(asio::io_context& ios) : patched_async_pipe(ios, ios) {} + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) inline patched_async_pipe(asio::io_context& ios_source, asio::io_context& ios_sink) : _source(ios_source), _sink(ios_sink) { int fds[2]; - if (::pipe(fds) == -1) + if (pipe(fds) == -1) boost::process::detail::throw_last_error("pipe(2) failed"); _source.assign(fds[0]); diff --git a/src/common/process.cpp b/src/common/process.cpp index 330440fe..817f706d 100644 --- a/src/common/process.cpp +++ b/src/common/process.cpp @@ -141,38 +141,39 @@ std::optional Process::Handle::wait() const noexcept { Process::Process(std::string command) : command_(command) {} Process::StringResult Process::spawn_get_stdout_line() const { - /// We'll read the results from a pipe. The child writes to the second pipe, - /// we'll read from the first one. - int output_pipe[2]; - ::pipe(output_pipe); + // We'll read the results from a pipe. The child writes to the second pipe, + // we'll read from the first one. + int stdout_pipe_fds[2]; + pipe(stdout_pipe_fds); const auto argv = build_argv(); const auto envp = env_ ? env_->make_environ() : environ; posix_spawn_file_actions_t actions; posix_spawn_file_actions_init(&actions); - posix_spawn_file_actions_adddup2(&actions, output_pipe[1], STDOUT_FILENO); + posix_spawn_file_actions_adddup2(&actions, stdout_pipe_fds[1], + STDOUT_FILENO); posix_spawn_file_actions_addopen(&actions, STDERR_FILENO, "/dev/null", O_WRONLY | O_APPEND, 0); - posix_spawn_file_actions_addclose(&actions, output_pipe[0]); - posix_spawn_file_actions_addclose(&actions, output_pipe[1]); + posix_spawn_file_actions_addclose(&actions, stdout_pipe_fds[0]); + posix_spawn_file_actions_addclose(&actions, stdout_pipe_fds[1]); pid_t child_pid = 0; const auto result = posix_spawnp(&child_pid, command_.c_str(), &actions, nullptr, argv, envp); - close(output_pipe[1]); + close(stdout_pipe_fds[1]); if (result == 2) { - close(output_pipe[0]); + close(stdout_pipe_fds[0]); return Process::CommandNotFound{}; } else if (result != 0) { - close(output_pipe[0]); + close(stdout_pipe_fds[0]); return std::error_code(result, std::system_category()); } // Try to read the first line out the output until the line feed std::array output{0}; - FILE* output_pipe_stream = fdopen(output_pipe[0], "r"); + FILE* output_pipe_stream = fdopen(stdout_pipe_fds[0], "r"); assert(output_pipe_stream); fgets(output.data(), output.size(), output_pipe_stream); fclose(output_pipe_stream); @@ -214,6 +215,117 @@ Process::StatusResult Process::spawn_get_status() const { } } +Process::HandleResult Process::spawn_child_piped( + // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) + asio::posix::stream_descriptor& stdout_pipe, + asio::posix::stream_descriptor& stderr_pipe) const { + // We'll reopen the child process' STDOUT and STDERR stream from a pipe, and + // we'll assign the other ends of those pipes to the stream descriptors + // passed to this function so they can be read from asynchronously in an + // Asio IO context loop. We'll read from the first elements of these pipes, + // and the child process will write to the second elements. + int stdout_pipe_fds[2]; + int stderr_pipe_fds[2]; + pipe(stdout_pipe_fds); + pipe(stderr_pipe_fds); + + const auto argv = build_argv(); + const auto envp = env_ ? env_->make_environ() : environ; + + posix_spawn_file_actions_t actions; + posix_spawn_file_actions_init(&actions); + posix_spawn_file_actions_adddup2(&actions, stdout_pipe_fds[1], + STDOUT_FILENO); + posix_spawn_file_actions_adddup2(&actions, stderr_pipe_fds[1], + STDERR_FILENO); + // We'll close the four pipe fds along with the rest of the file descriptors + +// NOTE: If the Wine process outlives the host, then it may cause issues if +// our process is still keeping the host's file descriptors alive +// that. This can prevent Ardour from restarting after an unexpected +// shutdown. Because of this we won't use `vfork()`, but instead we'll +// just manually close all non-STDIO file descriptors. +#if (__GLIBC__ > 2) || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 34) + posix_spawn_file_actions_addclosefrom_np(&actions, STDERR_FILENO + 1); +#else + const int max_fds = static_cast(sysconf(_SC_OPEN_MAX)); + for (int fd = STDERR_FILENO + 1; fd < max_fds; fd++) { + posix_spawn_file_actions_addclose(&actions, fd); + } +#endif + + pid_t child_pid = 0; + const auto result = posix_spawnp(&child_pid, command_.c_str(), &actions, + nullptr, argv, envp); + + // We'll assign the read ends of the pipes to the Asio stream descriptors + // passed to this function, even if launching the process failed. + // `asio::posix::stream_descriptor::assign()` will take ownership of the FD + // and close it when the object gets dropped. + stdout_pipe.assign(stdout_pipe_fds[0]); + stderr_pipe.assign(stderr_pipe_fds[0]); + close(stdout_pipe_fds[1]); + close(stderr_pipe_fds[1]); + + if (result == 2) { + return Process::CommandNotFound{}; + } else if (result != 0) { + return std::error_code(result, std::system_category()); + } + + // With glibc `posix_spawn*()` will return 2/`ENOENT` when the file does not + // exist, but the specification says that it should return a PID that exits + // with status 127 instead. I have no idea how we'd check for that without + // waiting here though, so this check may not work + int status = 0; + assert(waitpid(child_pid, &status, WNOHANG) >= 0); + if (WIFEXITED(status) && WEXITSTATUS(status) == 127) { + return Process::CommandNotFound{}; + } else { + return Handle(child_pid); + } +} + +Process::HandleResult Process::spawn_child_redirected( + const ghc::filesystem::path& filename) const { + const auto argv = build_argv(); + const auto envp = env_ ? env_->make_environ() : environ; + + posix_spawn_file_actions_t actions; + posix_spawn_file_actions_init(&actions); + posix_spawn_file_actions_addopen(&actions, STDOUT_FILENO, filename.c_str(), + O_WRONLY | O_APPEND, 0); + posix_spawn_file_actions_addopen(&actions, STDERR_FILENO, filename.c_str(), + O_WRONLY | O_APPEND, 0); + + // See the note in the other function +#if (__GLIBC__ > 2) || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 34) + posix_spawn_file_actions_addclosefrom_np(&actions, STDERR_FILENO + 1); +#else + const int max_fds = static_cast(sysconf(_SC_OPEN_MAX)); + for (int fd = STDERR_FILENO + 1; fd < max_fds; fd++) { + posix_spawn_file_actions_addclose(&actions, fd); + } +#endif + + pid_t child_pid = 0; + const auto result = posix_spawnp(&child_pid, command_.c_str(), &actions, + nullptr, argv, envp); + if (result == 2) { + return Process::CommandNotFound{}; + } else if (result != 0) { + return std::error_code(result, std::system_category()); + } + + int status = 0; + assert(waitpid(child_pid, &status, WNOHANG) >= 0); + if (WIFEXITED(status) && WEXITSTATUS(status) == 127) { + return Process::CommandNotFound{}; + } else { + return Handle(child_pid); + } +} + char* const* Process::build_argv() const { argv_.clear(); diff --git a/src/common/process.h b/src/common/process.h index 43ef993c..05354078 100644 --- a/src/common/process.h +++ b/src/common/process.h @@ -94,7 +94,8 @@ class ProcessEnvironment { /** * A child process whose output can be captured. Simple wrapper around the Posix - * APIs. + * APIs. The functions provided for running processes this way are very much + * tailored towards yabridge's needs. */ class Process { public: @@ -110,6 +111,8 @@ class Process { protected: Handle(pid_t pid); + friend Process; + public: /** * Terminates the process when it gets dropped. @@ -195,6 +198,26 @@ class Process { */ StatusResult spawn_get_status() const; + /** + * Spawn the process without waiting for its completion, leave STDIN alone, + * create pipes for STDOUT and STDERR, and assign those to the provided + * (empty) stream descriptors. Use `posix_spawn()`, closes all non-STDIO + * file descriptors. The process will be terminated when the child process + * handle gets dropped. + */ + HandleResult spawn_child_piped( + asio::posix::stream_descriptor& stdout_pipe, + asio::posix::stream_descriptor& stderr_pipe) const; + + /** + * Spawn the process without waiting for its completion, leave STDIN alone, + * and redirect STDOUT and STDERR to a file. Use `posix_spawn()`, closes all + * non-STDIO file descriptors. The process will be terminated when the child + * process handle gets dropped. + */ + HandleResult spawn_child_redirected( + const ghc::filesystem::path& filename) const; + private: /** * Create the `argv` array from the command and the arguments. Only valid as diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 1d0ec489..e8c3f512 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -64,7 +64,7 @@ StdIoCapture::StdIoCapture(asio::io_context& io_context, int file_descriptor) original_fd_copy_(dup(file_descriptor)) { // We'll use the second element of these two file descriptors to reopen // `file_descriptor`, and the first one to read the captured contents from - if (::pipe(pipe_fd_) != 0) { + if (pipe(pipe_fd_) != 0) { std::cerr << "Could not create pipe" << std::endl; throw std::system_error(errno, std::system_category()); } diff --git a/src/wine-host/xdnd-proxy.h b/src/wine-host/xdnd-proxy.h index 131ad343..08d56b28 100644 --- a/src/wine-host/xdnd-proxy.h +++ b/src/wine-host/xdnd-proxy.h @@ -105,6 +105,8 @@ class WineXdndProxy { */ Handle(WineXdndProxy* proxy); + friend WineXdndProxy; + public: /** * Reduces the reference count by one, and frees `proxy_` if this was @@ -120,8 +122,6 @@ class WineXdndProxy { private: WineXdndProxy* proxy_; - - friend WineXdndProxy; }; /**