Subprocesses can start as heads of their own process group
Bug: 140224512
Test: locally
Change-Id: I83e3888cf0e0741b9182399082d6f20b78d81bfc
diff --git a/common/libs/utils/subprocess.cpp b/common/libs/utils/subprocess.cpp
index bc24cdc..9ea4626 100644
--- a/common/libs/utils/subprocess.cpp
+++ b/common/libs/utils/subprocess.cpp
@@ -40,10 +40,10 @@
// Add the redirected IO channels to a set as integers. This allows converting
// the enum values into integers instead of the other way around.
std::set<int> int_redirects;
- for (const auto& entry: redirects) {
+ for (const auto& entry : redirects) {
int_redirects.insert(static_cast<int>(entry.first));
}
- for (const auto& entry: inherited_fds) {
+ for (const auto& entry : inherited_fds) {
auto dupped_fd = entry.second;
if (int_redirects.count(dupped_fd)) {
LOG(ERROR) << "Requested redirect of fd(" << dupped_fd
@@ -54,8 +54,9 @@
return true;
}
-void do_redirects(const std::map<cvd::Subprocess::StdIOChannel, int>& redirects) {
- for (const auto& entry: redirects) {
+void do_redirects(
+ const std::map<cvd::Subprocess::StdIOChannel, int>& redirects) {
+ for (const auto& entry : redirects) {
auto std_channel = static_cast<int>(entry.first);
auto fd = entry.second;
TEMP_FAILURE_RETRY(dup2(fd, std_channel));
@@ -63,11 +64,10 @@
}
cvd::Subprocess subprocess_impl(
- const char* const* command,
- const char* const* envp,
+ const char* const* command, const char* const* envp,
const std::map<cvd::Subprocess::StdIOChannel, int>& redirects,
- const std::map<cvd::SharedFD, int>& inherited_fds,
- bool with_control_socket) {
+ const std::map<cvd::SharedFD, int>& inherited_fds, bool with_control_socket,
+ bool in_group = false) {
// The parent socket will get closed on the child on the call to exec, the
// child socket will be closed on the parent when this function returns and no
// references to the fd are left
@@ -90,6 +90,13 @@
pid_t pid = fork();
if (!pid) {
do_redirects(redirects);
+ if (in_group) {
+ // This call should never fail (see SETPGID(2))
+ if (setpgid(0, 0) != 0) {
+ auto error = errno;
+ LOG(ERROR) << "setpgid failed (" << strerror(error) << ")";
+ }
+ }
int rval;
// If envp is NULL, the current process's environment is used as the
// environment of the child process. To force an empty emvironment for
@@ -97,8 +104,7 @@
if (envp == NULL) {
rval = execv(command[0], const_cast<char* const*>(command));
} else {
- rval = execve(command[0],
- const_cast<char* const*>(command),
+ rval = execve(command[0], const_cast<char* const*>(command),
const_cast<char* const*>(envp));
}
// No need for an if: if exec worked it wouldn't have returned
@@ -117,8 +123,7 @@
return cvd::Subprocess(pid, parent_socket);
}
-std::vector<const char*> ToCharPointers(
- const std::vector<std::string>& vect) {
+std::vector<const char*> ToCharPointers(const std::vector<std::string>& vect) {
std::vector<const char*> ret = {};
for (const auto& str : vect) {
ret.push_back(str.c_str());
@@ -158,7 +163,7 @@
return -1;
}
int wstatus = 0;
- auto pid = pid_; // Wait will set pid_ to -1 after waiting
+ auto pid = pid_; // Wait will set pid_ to -1 after waiting
auto wait_ret = Wait(&wstatus, 0);
if (wait_ret < 0) {
LOG(ERROR) << "Error on call to waitpid: " << strerror(errno);
@@ -191,10 +196,8 @@
return retval;
}
-Command::ParameterBuilder::~ParameterBuilder() {
- Build();
-}
-void Command::ParameterBuilder::Build() {
+Command::ParameterBuilder::~ParameterBuilder() { Build(); }
+void Command::ParameterBuilder::Build() {
auto param = stream_.str();
stream_ = std::stringstream();
if (param.size()) {
@@ -204,11 +207,11 @@
Command::~Command() {
// Close all inherited file descriptors
- for(const auto& entry: inherited_fds_) {
+ for (const auto& entry : inherited_fds_) {
close(entry.second);
}
// Close all redirected file descriptors
- for (const auto& entry: redirects_) {
+ for (const auto& entry : redirects_) {
close(entry.second);
}
}
@@ -229,7 +232,7 @@
}
bool Command::RedirectStdIO(cvd::Subprocess::StdIOChannel channel,
- cvd::SharedFD shared_fd){
+ cvd::SharedFD shared_fd) {
if (!shared_fd->IsOpen()) {
return false;
}
@@ -251,18 +254,26 @@
cvd::SharedFD::Dup(static_cast<int>(parent_channel)));
}
-Subprocess Command::Start(bool with_control_socket) const {
+Subprocess Command::StartHelper(bool with_control_socket, bool in_group) const {
auto cmd = ToCharPointers(command_);
if (use_parent_env_) {
return subprocess_impl(cmd.data(), nullptr, redirects_, inherited_fds_,
- with_control_socket);
+ with_control_socket, in_group);
} else {
auto envp = ToCharPointers(env_);
return subprocess_impl(cmd.data(), envp.data(), redirects_, inherited_fds_,
- with_control_socket);
+ with_control_socket, in_group);
}
}
+Subprocess Command::Start(bool with_control_socket) const {
+ return StartHelper(with_control_socket, false);
+}
+
+Subprocess Command::StartInGroup(bool with_control_socket) const {
+ return StartHelper(with_control_socket, true);
+}
+
int execute(const std::vector<std::string>& command,
const std::vector<std::string>& env) {
Command cmd(command[0]);
diff --git a/common/libs/utils/subprocess.h b/common/libs/utils/subprocess.h
index ee08574..e5b6352 100644
--- a/common/libs/utils/subprocess.h
+++ b/common/libs/utils/subprocess.h
@@ -18,8 +18,8 @@
#include <sys/types.h>
#include <map>
-#include <string>
#include <sstream>
+#include <string>
#include <vector>
#include <common/libs/fs/shared_fd.h>
@@ -50,10 +50,9 @@
// Whether the command started successfully. It only says whether the call to
// fork() succeeded or not, it says nothing about exec or successful
// completion of the command, that's what Wait is for.
- bool Started() const {return started_;}
- SharedFD control_socket() {
- return control_socket_;
- }
+ bool Started() const { return started_; }
+ SharedFD control_socket() { return control_socket_; }
+ pid_t pid() const { return pid_; }
private:
// Copy is disabled to avoid waiting twice for the same pid (the first wait
@@ -71,7 +70,7 @@
// should inherit.
class Command {
private:
- template<typename T>
+ template <typename T>
// For every type other than SharedFD (for which there is a specialisation)
bool BuildParameter(std::stringstream* stream, T t) {
*stream << t;
@@ -79,33 +78,32 @@
}
// Special treatment for SharedFD
bool BuildParameter(std::stringstream* stream, SharedFD shared_fd);
- template<typename T, typename...Args>
- bool BuildParameter(std::stringstream* stream, T t, Args...args) {
- return BuildParameter(stream, t) &&
- BuildParameter(stream, args...);
+ template <typename T, typename... Args>
+ bool BuildParameter(std::stringstream* stream, T t, Args... args) {
+ return BuildParameter(stream, t) && BuildParameter(stream, args...);
}
+
public:
class ParameterBuilder {
- public:
- ParameterBuilder(Command* cmd) : cmd_(cmd) {};
+ public:
+ ParameterBuilder(Command* cmd) : cmd_(cmd){};
ParameterBuilder(ParameterBuilder&& builder) = default;
~ParameterBuilder();
- template<typename T>
+ template <typename T>
ParameterBuilder& operator<<(T t) {
cmd_->BuildParameter(&stream_, t);
return *this;
}
void Build();
- private:
+
+ private:
cvd::Command* cmd_;
std::stringstream stream_;
};
- Command(const std::string& executable) {
- command_.push_back(executable);
- }
+ Command(const std::string& executable) { command_.push_back(executable); }
Command(Command&&) = default;
// The default copy constructor is unsafe because it would mean multiple
// closing of the inherited file descriptors. If needed it can be implemented
@@ -125,7 +123,7 @@
// SharedFD a duplicate of it will be used and won't be closed until the
// object is destroyed. To add multiple parameters to the command the function
// must be called multiple times, one per parameter.
- template<typename... Args>
+ template <typename... Args>
bool AddParameter(Args... args) {
std::stringstream ss;
if (BuildParameter(&ss, args...)) {
@@ -135,9 +133,7 @@
return false;
}
- ParameterBuilder GetParameterBuilder() {
- return ParameterBuilder(this);
- }
+ ParameterBuilder GetParameterBuilder() { return ParameterBuilder(this); }
// Redirects the standard IO of the command.
bool RedirectStdIO(Subprocess::StdIOChannel channel, cvd::SharedFD shared_fd);
@@ -149,13 +145,19 @@
// with_control_socket is true the returned Subprocess instance will have a
// sharedFD that enables communication with the child process.
Subprocess Start(bool with_control_socket = false) const;
+ // Same as Start(bool), but the subprocess runs as head of its own process
+ // group.
+ Subprocess StartInGroup(bool with_control_socket = false) const;
std::string GetShortName() const {
// This is safe because the constructor guarantees the name of the binary to
// be at index 0 on the vector
return command_[0];
}
+
private:
+ Subprocess StartHelper(bool with_control_socket, bool in_group) const;
+
std::vector<std::string> command_;
std::map<cvd::SharedFD, int> inherited_fds_{};
std::map<Subprocess::StdIOChannel, int> redirects_{};