Commit 188e13f3 authored by Alexey Spiridonov's avatar Alexey Spiridonov Committed by Sara Golemon

Change child's working directory

Summary: Changing the parent's WD is prone to race conditions of various sorts, and needlessly alters parent state. Python's subprocess also has this feature.

Test Plan: fbmake dbg _bin/folly/test/subprocess_test ; ../_bin/folly/test/subprocess_test

Reviewed By: agoder@fb.com

FB internal diff: D1269526
parent a6e257a5
......@@ -413,6 +413,14 @@ int Subprocess::prepareChild(const Options& options,
return r; // pthread_sigmask() returns an errno value
}
// Change the working directory, if one is given
if (!options.childDir_.empty()) {
r = ::chdir(options.childDir_.c_str());
if (r == -1) {
return errno;
}
}
// Close parent's ends of all pipes
for (auto& p : pipes_) {
r = ::close(p.parentFd);
......
......@@ -264,6 +264,11 @@ class Subprocess : private boost::noncopyable {
*/
Options& usePath() { usePath_ = true; return *this; }
/**
* Change the child's working directory, after the vfork.
*/
Options& chdir(const std::string& dir) { childDir_ = dir; return *this; }
#if __linux__
/**
* Child will receive a signal when the parent exits.
......@@ -284,6 +289,7 @@ class Subprocess : private boost::noncopyable {
FdMap fdActions_;
bool closeOtherFds_;
bool usePath_;
std::string childDir_; // "" keeps the parent's working directory
#if __linux__
int parentDeathSignal_{0};
#endif
......
......@@ -88,6 +88,33 @@ TEST(SimpleSubprocessTest, ShellExitsWithError) {
EXPECT_EQ(1, proc.wait().exitStatus());
}
TEST(SimpleSubprocessTest, ChangeChildDirectorySuccessfully) {
// The filesystem root normally lacks a 'true' binary
EXPECT_EQ(0, chdir("/"));
EXPECT_SPAWN_ERROR(ENOENT, "failed to execute ./true", "./true");
// The child can fix that by moving to /bin before exec().
Subprocess proc("./true", Subprocess::Options().chdir("/bin"));
EXPECT_EQ(0, proc.wait().exitStatus());
}
TEST(SimpleSubprocessTest, ChangeChildDirectoryWithError) {
try {
Subprocess proc(
std::vector<std::string>{"/bin/true"},
Subprocess::Options().chdir("/usually/this/is/not/a/valid/directory/")
);
ADD_FAILURE() << "expected to fail when changing the child's directory";
} catch (const SubprocessSpawnError& ex) {
EXPECT_EQ(ENOENT, ex.errnoValue());
const std::string expectedError =
"error preparing to execute /bin/true: No such file or directory";
if (StringPiece(ex.what()).find(expectedError) == StringPiece::npos) {
ADD_FAILURE() << "failed to find \"" << expectedError <<
"\" in exception: \"" << ex.what() << "\"";
}
}
}
namespace {
boost::container::flat_set<int> getOpenFds() {
auto pid = getpid();
......
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