Commit 8a5fcb4f authored by Alexey Spiridonov's avatar Alexey Spiridonov Committed by Dave Watson

Add process group leader option

Summary: Now a subprocess can be reliably made a group leader -- good for job control.

Test Plan:
unit test, checked that the pgid test worked in bash, too (only OKAY_XXX is printed)

==> XXX.sh <==
#!/bin/sh
test $(cut -d ' ' -f 5 /proc/$$/stat) == $$ && echo OKAY_XXX
./YYY.sh

==> YYY.sh <==
#!/bin/sh
test $(cut -d ' ' -f 5 /proc/$$/stat) == $$ && echo OKAY_YYY
./ZZZ.sh

==> ZZZ.sh <==
#!/bin/sh
test $(cut -d ' ' -f 5 /proc/$$/stat) == $$ && echo OKAY_ZZZ

Reviewed By: simpkins@fb.com

Subscribers: net-systems@, njormrod, folly-diffs@, simpkins

FB internal diff: D1492526

Signature: t1:1492526:1416528620:3cf98b1c1e334a7d551b2c2f3e76b1c70f07ad7c
parent 136dd179
...@@ -289,6 +289,14 @@ void Subprocess::spawn( ...@@ -289,6 +289,14 @@ void Subprocess::spawn(
// child has exited and can be immediately waited for. In all other cases, // child has exited and can be immediately waited for. In all other cases,
// we have no way of cleaning up the child. // we have no way of cleaning up the child.
if (options.processGroupLeader_) {
// This is done both in the parent and the child to avoid the race where
// the parent assumes that the child is a leader, but the child has not
// yet run setprp(). Not checking error codes since we're deliberately
// racing the child, which may already have run execve(), and expect to
// lose frequently.
setpgid(pid_, pid_);
}
// Close writable side of the errFd pipe in the parent process // Close writable side of the errFd pipe in the parent process
CHECK_ERR(::close(errFds[1])); CHECK_ERR(::close(errFds[1]));
errFds[1] = -1; errFds[1] = -1;
...@@ -486,6 +494,12 @@ int Subprocess::prepareChild(const Options& options, ...@@ -486,6 +494,12 @@ int Subprocess::prepareChild(const Options& options,
} }
#endif #endif
if (options.processGroupLeader_) {
if (setpgrp() == -1) {
return errno;
}
}
return 0; return 0;
} }
......
...@@ -205,10 +205,7 @@ class Subprocess : private boost::noncopyable { ...@@ -205,10 +205,7 @@ class Subprocess : private boost::noncopyable {
class Options : private boost::orable<Options> { class Options : private boost::orable<Options> {
friend class Subprocess; friend class Subprocess;
public: public:
Options() Options() {} // E.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58328
: closeOtherFds_(false),
usePath_(false) {
}
/** /**
* Change action for file descriptor fd. * Change action for file descriptor fd.
...@@ -281,6 +278,16 @@ class Subprocess : private boost::noncopyable { ...@@ -281,6 +278,16 @@ class Subprocess : private boost::noncopyable {
} }
#endif #endif
/**
* Child will be made a process group leader when it starts. Upside: one
* can reliably all its kill non-daemonizing descendants. Downside: the
* child will not receive Ctrl-C etc during interactive use.
*/
Options& processGroupLeader() {
processGroupLeader_ = true;
return *this;
}
/** /**
* Helpful way to combine Options. * Helpful way to combine Options.
*/ */
...@@ -289,12 +296,13 @@ class Subprocess : private boost::noncopyable { ...@@ -289,12 +296,13 @@ class Subprocess : private boost::noncopyable {
private: private:
typedef boost::container::flat_map<int, int> FdMap; typedef boost::container::flat_map<int, int> FdMap;
FdMap fdActions_; FdMap fdActions_;
bool closeOtherFds_; bool closeOtherFds_{false};
bool usePath_; bool usePath_{false};
std::string childDir_; // "" keeps the parent's working directory std::string childDir_; // "" keeps the parent's working directory
#if __linux__ #if __linux__
int parentDeathSignal_{0}; int parentDeathSignal_{0};
#endif #endif
bool processGroupLeader_{false};
}; };
static Options pipeStdin() { return Options().stdin(PIPE); } static Options pipeStdin() { return Options().stdin(PIPE); }
...@@ -660,6 +668,7 @@ inline Subprocess::Options& Subprocess::Options::operator|=( ...@@ -660,6 +668,7 @@ inline Subprocess::Options& Subprocess::Options::operator|=(
} }
closeOtherFds_ |= other.closeOtherFds_; closeOtherFds_ |= other.closeOtherFds_;
usePath_ |= other.usePath_; usePath_ |= other.usePath_;
processGroupLeader_ |= other.processGroupLeader_;
return *this; return *this;
} }
......
...@@ -256,6 +256,14 @@ TEST(CommunicateSubprocessTest, Duplex) { ...@@ -256,6 +256,14 @@ TEST(CommunicateSubprocessTest, Duplex) {
proc.waitChecked(); proc.waitChecked();
} }
TEST(CommunicateSubprocessTest, ProcessGroupLeader) {
const auto testIsLeader = "test $(cut -d ' ' -f 5 /proc/$$/stat) == $$";
Subprocess nonLeader(testIsLeader);
EXPECT_THROW(nonLeader.waitChecked(), CalledProcessError);
Subprocess leader(testIsLeader, Subprocess::Options().processGroupLeader());
leader.waitChecked();
}
TEST(CommunicateSubprocessTest, Duplex2) { TEST(CommunicateSubprocessTest, Duplex2) {
checkFdLeak([] { checkFdLeak([] {
// Pipe 200,000 lines through sed // Pipe 200,000 lines through sed
......
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