audacity/linuxdeploy
1
0
Fork 0
mirror of https://github.com/audacity/linuxdeploy.git synced 2025-12-10 03:56:50 -06:00
Code Issues Packages Projects Releases 1 Wiki Activity

Handle end of child processes correctly

Browse Source 
Fixes a bug introduced previously where linuxdeploy cannot properly detect a child process exited.
This commit is contained in:
TheAssassin 2022-07-14 12:31:36 +02:00
parent aa203927c7
commit 649fc0247d
3 changed files with 111 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
include/linuxdeploy/subprocess/process.h
View File

@ -25,14 +25,6 @@ namespace linuxdeploy {
// these constants help make the pipe code more readable
static constexpr int READ_END_ = 0, WRITE_END_ = 1;
static std::vector<char*> make_args_vector_(const std::vector<std::string>& args);
static std::vector<char*> make_env_vector_(const subprocess_env_map_t& env);
static int check_waitpid_status_(int status);
static void close_pipe_fd_(int fd);
public:
/**
* Create a child process.

8
src/plugin/plugin_process_handler.cpp
View File

@ -58,15 +58,15 @@ namespace linuxdeploy {
for (;;) {
for (auto& pipe_to_be_logged : pipes_to_be_logged) {
if (pipe_to_be_logged.eof) {
continue;
}
const auto log_prefix = "[" + name_ + "/" + pipe_to_be_logged.stream_name_ + "] ";
// since we have our own ldLog instance for every pipe, we can get away with this rather small read buffer
subprocess::subprocess_result_buffer_t intermediate_buffer(4096);
if (pipe_to_be_logged.eof) {
break;
}
// (try to) read from pipe
const auto bytes_read = pipe_to_be_logged.reader_.read(intermediate_buffer);

215
src/subprocess/process.cpp
View File

@ -17,6 +17,91 @@
// shorter than using namespace ...
using namespace linuxdeploy::subprocess;
namespace {
std::vector<char*> make_args_vector(const std::vector<std::string>& args) {
std::vector<char*> rv{};
rv.reserve(args.size());
for (const auto& arg : args) {
rv.emplace_back(strdup(arg.c_str()));
}
// execv* want a nullptr-terminated array
rv.emplace_back(nullptr);
return rv;
}
std::vector<char*> make_env_vector(const subprocess_env_map_t& env) {
std::vector<char*> rv;
// first, copy existing environment
// we cannot reserve space in the vector unfortunately, as we don't know the size of environ before the iteration
if (environ != nullptr) {
for (auto** current_env_var = environ; *current_env_var != nullptr; ++current_env_var) {
rv.emplace_back(strdup(*current_env_var));
}
}
// add own environment variables, overwriting existing ones if necessary
for (const auto& env_var : env) {
const auto& key = env_var.first;
const auto& value = env_var.second;
auto predicate = [&key](char* existing_env_var) {
char* equal_sign = strstr(existing_env_var, "=");
if (equal_sign == nullptr) {
throw std::runtime_error{"no equal sign in environment variable"};
}
return strncmp(existing_env_var, key.c_str(), std::distance(equal_sign, existing_env_var)) == 0;
};
// delete existing env var, if any
rv.erase(std::remove_if(rv.begin(), rv.end(), predicate), rv.end());
// insert new value
std::ostringstream oss;
oss << key;
oss << "=";
oss << value;
rv.emplace_back(strdup(oss.str().c_str()));
}
// exec*e want a nullptr-terminated array
rv.emplace_back(nullptr);
return rv;
}
int check_waitpid_status(int status) {
if (WIFSIGNALED(status) != 0) {
// TODO: consider treating child exit caused by signals separately
return WTERMSIG(status);
} else if (WIFEXITED(status) != 0) {
return WEXITSTATUS(status);
}
throw std::logic_error{"unknown child process state"};
}
void close_pipe_fd(int fd, bool ignore_ebadf = false) {
const auto rv = ::close(fd);
if (rv != 0) {
const auto error = errno;
// might be closed already by the child process, in this case we can safely ignore the error
if (error != EBADF) {
throw std::logic_error("failed to close pipe fd: " + std::string(strerror(error)));
};
}
}
}
int process::pid() const {
return child_pid_;
}
@ -66,8 +151,8 @@ process::process(const std::vector<std::string>& args, const subprocess_env_map_
// we're in the child process
// first step: close the read end of both pipes
close_pipe_fd_(stdout_pipe_fds[READ_END_]);
close_pipe_fd_(stderr_pipe_fds[READ_END_]);
close_pipe_fd(stdout_pipe_fds[READ_END_]);
close_pipe_fd(stderr_pipe_fds[READ_END_]);
auto connect_fd = [](int fd, int fileno) {
for (;;) {
@ -87,12 +172,12 @@ process::process(const std::vector<std::string>& args, const subprocess_env_map_
connect_fd(stderr_pipe_fds[WRITE_END_], STDERR_FILENO);
// now, we also have to close the write end of both pipes
close_pipe_fd_(stdout_pipe_fds[WRITE_END_]);
close_pipe_fd_(stderr_pipe_fds[WRITE_END_]);
close_pipe_fd(stdout_pipe_fds[WRITE_END_]);
close_pipe_fd(stderr_pipe_fds[WRITE_END_]);
// prepare arguments for exec*
auto exec_args = make_args_vector_(args);
auto exec_env = make_env_vector_(env);
auto exec_args = make_args_vector(args);
auto exec_env = make_env_vector(env);
// call subprocess
execvpe(args.front().c_str(), exec_args.data(), exec_env.data());
@ -114,9 +199,9 @@ process::process(const std::vector<std::string>& args, const subprocess_env_map_
// parent code
// we do not intend to write to the processes
close_pipe_fd_(stdout_pipe_fds[WRITE_END_]);
close_pipe_fd_(stderr_pipe_fds[WRITE_END_]);
// we do not intend to write to these pipes from this end
close_pipe_fd(stdout_pipe_fds[WRITE_END_]);
close_pipe_fd(stderr_pipe_fds[WRITE_END_]);
// store file descriptors
stdout_fd_ = stdout_pipe_fds[READ_END_];
@ -125,24 +210,22 @@ process::process(const std::vector<std::string>& args, const subprocess_env_map_
int process::close() {
if (!exited_) {
close_pipe_fd_(stdout_fd_);
stdout_fd_ = -1;
int status;
close_pipe_fd_(stderr_fd_);
stderr_fd_ = -1;
{
int status;
if (waitpid(child_pid_, &status, 0) == -1) {
throw std::logic_error{"waitpid() failed"};
}
exited_ = true;
exit_code_ = check_waitpid_status_(status);
if (waitpid(child_pid_, &status, 0) == -1) {
throw std::logic_error{"waitpid() failed"};
}
exited_ = true;
exit_code_ = check_waitpid_status(status);
}
close_pipe_fd(stdout_fd_, true);
stdout_fd_ = -1;
close_pipe_fd(stderr_fd_, true);
stderr_fd_ = -1;
return exit_code_;
}
@ -150,63 +233,6 @@ process::~process() {
(void) close();
}
std::vector<char*> process::make_args_vector_(const std::vector<std::string>& args) {
std::vector<char*> rv{};
rv.reserve(args.size());
for (const auto& arg : args) {
rv.emplace_back(strdup(arg.c_str()));
}
// execv* want a nullptr-terminated array
rv.emplace_back(nullptr);
return rv;
}
std::vector<char*> process::make_env_vector_(const subprocess_env_map_t& env) {
std::vector<char*> rv;
// first, copy existing environment
// we cannot reserve space in the vector unfortunately, as we don't know the size of environ before the iteration
if (environ != nullptr) {
for (auto** current_env_var = environ; *current_env_var != nullptr; ++current_env_var) {
rv.emplace_back(strdup(*current_env_var));
}
}
// add own environment variables, overwriting existing ones if necessary
for (const auto& env_var : env) {
const auto& key = env_var.first;
const auto& value = env_var.second;
auto predicate = [&key](char* existing_env_var) {
char* equal_sign = strstr(existing_env_var, "=");
if (equal_sign == nullptr) {
throw std::runtime_error{"no equal sign in environment variable"};
}
return strncmp(existing_env_var, key.c_str(), std::distance(equal_sign, existing_env_var)) == 0;
};
// delete existing env var, if any
rv.erase(std::remove_if(rv.begin(), rv.end(), predicate), rv.end());
// insert new value
std::ostringstream oss;
oss << key;
oss << "=";
oss << value;
rv.emplace_back(strdup(oss.str().c_str()));
}
// exec*e want a nullptr-terminated array
rv.emplace_back(nullptr);
return rv;
}
void process::kill(int signal) const {
if (::kill(child_pid_, signal) != 0) {
@ -231,15 +257,8 @@ bool process::is_running() {
}
if (result == child_pid_) {
// TODO: extract the following lines from both this method and close() to eliminate duplicate code
close_pipe_fd_(stdout_fd_);
stdout_fd_ = -1;
close_pipe_fd_(stderr_fd_);
stderr_fd_ = -1;
exited_ = true;
exit_code_ = check_waitpid_status_(status);
exit_code_ = check_waitpid_status(status);
return false;
}
@ -252,23 +271,3 @@ bool process::is_running() {
// can only happen if waitpid() returns an unknown process ID
throw std::logic_error{"unknown error occured"};
}
int process::check_waitpid_status_(int status) {
if (WIFSIGNALED(status) != 0) {
// TODO: consider treating child exit caused by signals separately
return WTERMSIG(status);
} else if (WIFEXITED(status) != 0) {
return WEXITSTATUS(status);
}
throw std::logic_error{"unknown child process state"};
}
void process::close_pipe_fd_(int fd) {
const auto rv = ::close(fd);
if (rv != 0) {
const auto error = errno;
throw std::logic_error("failed to close pipe fd: " + std::string(strerror(error)));
}
}