Commit 2b0923c6 authored by Adam Simpkins's avatar Adam Simpkins Committed by Facebook Github Bot

add a `detach` option to `Subprocess` to avoid waiting for the child

Summary:
Add a `Subprocess::Options::detach()` call.  This causes the subprocess to be
spawned in grandchild process that will be reparented to init.  The
intermediate child process exits immediately after spawning the grandchild,
allowing the parent process to `wait()` on it and destroy its `Subprocess`
object without leaving any zombie children.

Reviewed By: yfeldblum

Differential Revision: D10860274

fbshipit-source-id: ac27d77d0033e3c03e27dd3f758ab70fd2dd990f
parent d188bd70
......@@ -308,6 +308,12 @@ void Subprocess::spawn(
// calling exec()
readChildErrorPipe(errFds[0], executable);
// If we spawned a detached child, wait on the intermediate child process.
// It always exits immediately.
if (options.detach_) {
wait();
}
// We have fully succeeded now, so release the guard on pipes_
pipesGuard.dismiss();
}
......@@ -425,15 +431,45 @@ void Subprocess::spawnInternal(
#ifdef __linux__
if (options.cloneFlags_) {
pid = syscall(SYS_clone, *options.cloneFlags_, 0, nullptr, nullptr);
checkUnixError(pid, errno, "clone");
} else {
#endif
if (options.detach_) {
// If we are detaching we must use fork() instead of vfork() for the first
// fork, since we aren't going to simply call exec() in the child.
pid = fork();
} else {
pid = vfork();
checkUnixError(pid, errno, "vfork");
}
#ifdef __linux__
}
#endif
checkUnixError(pid, errno, "failed to fork");
if (pid == 0) {
// Fork a second time if detach_ was requested.
// This must be done before signals are restored in prepareChild()
if (options.detach_) {
#ifdef __linux__
if (options.cloneFlags_) {
pid = syscall(SYS_clone, *options.cloneFlags_, 0, nullptr, nullptr);
} else {
#endif
pid = vfork();
#ifdef __linux__
}
#endif
if (pid == -1) {
// Inform our parent process of the error so it can throw in the parent.
childError(errFd, kChildFailure, errno);
} else if (pid != 0) {
// We are the intermediate process. Exit immediately.
// Our child will still inform the original parent of success/failure
// through errFd. The pid of the grandchild process never gets
// propagated back up to the original parent. In the future we could
// potentially send it back using errFd if we needed to.
_exit(0);
}
}
int errnoValue = prepareChild(options, &oldSignals, childDir);
if (errnoValue != 0) {
childError(errFd, kChildFailure, errnoValue);
......
......@@ -406,6 +406,23 @@ class Subprocess {
return *this;
}
/**
* Detach the spawned process, to allow destroying the Subprocess object
* without waiting for the child process to finish.
*
* This causes the code to fork twice before executing the command.
* The intermediate child process will exit immediately, causing the process
* running the executable to be reparented to init (pid 1).
*
* Subprocess objects created with detach() enabled will already be in an
* "EXITED" state when the constructor returns. The caller should not call
* wait() or poll() on the Subprocess, and pid() will return -1.
*/
Options& detach() {
detach_ = true;
return *this;
}
/**
* *** READ THIS WHOLE DOCBLOCK BEFORE USING ***
*
......@@ -471,11 +488,12 @@ class Subprocess {
FdMap fdActions_;
bool closeOtherFds_{false};
bool usePath_{false};
bool processGroupLeader_{false};
bool detach_{false};
std::string childDir_; // "" keeps the parent's working directory
#if __linux__
int parentDeathSignal_{0};
#endif
bool processGroupLeader_{false};
DangerousPostForkPreExecCallback* dangerousPostForkPreExecCallback_{
nullptr};
#if __linux__
......
......@@ -17,6 +17,7 @@
#include <folly/Subprocess.h>
#include <sys/types.h>
#include <chrono>
#include <boost/container/flat_set.hpp>
#include <glog/logging.h>
......@@ -36,6 +37,7 @@
FOLLY_GNU_DISABLE_WARNING("-Wdeprecated-declarations")
using namespace folly;
using namespace std::chrono_literals;
TEST(SimpleSubprocessTest, ExitsSuccessfully) {
Subprocess proc(std::vector<std::string>{"/bin/true"});
......@@ -108,10 +110,11 @@ TEST(SimpleSubprocessTest, DefaultConstructor) {
EXPECT_EQ(0, proc.wait().exitStatus());
}
#define EXPECT_SPAWN_ERROR(err, errMsg, cmd, ...) \
#define EXPECT_SPAWN_OPT_ERROR(err, errMsg, options, cmd, ...) \
do { \
try { \
Subprocess proc(std::vector<std::string>{(cmd), ##__VA_ARGS__}); \
Subprocess proc( \
std::vector<std::string>{(cmd), ##__VA_ARGS__}, (options)); \
ADD_FAILURE() << "expected an error when running " << (cmd); \
} catch (const SubprocessSpawnError& ex) { \
EXPECT_EQ((err), ex.errnoValue()); \
......@@ -122,6 +125,9 @@ TEST(SimpleSubprocessTest, DefaultConstructor) {
} \
} while (0)
#define EXPECT_SPAWN_ERROR(err, errMsg, cmd, ...) \
EXPECT_SPAWN_OPT_ERROR(err, errMsg, Subprocess::Options(), cmd, ##__VA_ARGS__)
TEST(SimpleSubprocessTest, ExecFails) {
EXPECT_SPAWN_ERROR(
ENOENT, "failed to execute /no/such/file:", "/no/such/file");
......@@ -230,6 +236,32 @@ TEST(SimpleSubprocessTest, FdLeakTest) {
});
}
TEST(SimpleSubprocessTest, Detach) {
auto start = std::chrono::steady_clock::now();
{
Subprocess proc(
std::vector<std::string>{"/bin/sleep", "10"},
Subprocess::Options().detach());
EXPECT_EQ(-1, proc.pid());
}
auto end = std::chrono::steady_clock::now();
// We should be able to create and destroy the Subprocess object quickly,
// without waiting for the sleep process to finish. This should usually
// happen in a matter of milliseconds, but we allow up to 5 seconds just to
// provide lots of leeway on heavily loaded continuous build machines.
EXPECT_LE(end - start, 5s);
}
TEST(SimpleSubprocessTest, DetachExecFails) {
// Errors executing the process should be propagated from the grandchild
// process back to the original parent process.
EXPECT_SPAWN_OPT_ERROR(
ENOENT,
"failed to execute /no/such/file:",
Subprocess::Options().detach(),
"/no/such/file");
}
TEST(ParentDeathSubprocessTest, ParentDeathSignal) {
// Find out where we are.
const auto basename = "subprocess_test_parent_death_helper";
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment