Needless temporary gone
[folly.git] / folly / Subprocess.cpp
index 4cd2c7c1088b0021d2244477fd03134c2cf29e82..7ecc8de486634aa315187438808b9a358eb5393d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 Facebook, Inc.
+ * Copyright 2015 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -50,6 +50,18 @@ constexpr int kChildFailure = 126;
 
 namespace folly {
 
+ProcessReturnCode::ProcessReturnCode(ProcessReturnCode&& p) noexcept
+  : rawStatus_(p.rawStatus_) {
+  p.rawStatus_ = ProcessReturnCode::RV_NOT_STARTED;
+}
+
+ProcessReturnCode& ProcessReturnCode::operator=(ProcessReturnCode&& p)
+    noexcept {
+  rawStatus_ = p.rawStatus_;
+  p.rawStatus_ = ProcessReturnCode::RV_NOT_STARTED;
+  return *this;
+}
+
 ProcessReturnCode::State ProcessReturnCode::state() const {
   if (rawStatus_ == RV_NOT_STARTED) return NOT_STARTED;
   if (rawStatus_ == RV_RUNNING) return RUNNING;
@@ -96,6 +108,7 @@ std::string ProcessReturnCode::str() const {
                            (coreDumped() ? " (core dumped)" : ""));
   }
   CHECK(false);  // unreached
+  return "";  // silence GCC warning
 }
 
 CalledProcessError::CalledProcessError(ProcessReturnCode rc)
@@ -118,7 +131,7 @@ namespace {
 // Copy pointers to the given strings in a format suitable for posix_spawn
 std::unique_ptr<const char*[]> cloneStrings(const std::vector<std::string>& s) {
   std::unique_ptr<const char*[]> d(new const char*[s.size() + 1]);
-  for (int i = 0; i < s.size(); i++) {
+  for (size_t i = 0; i < s.size(); i++) {
     d[i] = s[i].c_str();
   }
   d[s.size()] = nullptr;
@@ -189,13 +202,9 @@ Subprocess::Subprocess(
 Subprocess::~Subprocess() {
   CHECK_NE(returnCode_.state(), ProcessReturnCode::RUNNING)
     << "Subprocess destroyed without reaping child";
-  closeAll();
 }
 
 namespace {
-void closeChecked(int fd) {
-  checkUnixError(::close(fd), "close");
-}
 
 struct ChildErrorInfo {
   int errCode;
@@ -214,16 +223,9 @@ void childError(int errFd, int errCode, int errnoValue) {
 
 }  // namespace
 
-void Subprocess::closeAll() {
-  for (auto& p : pipes_) {
-    closeChecked(p.parentFd);
-  }
-  pipes_.clear();
-}
-
 void Subprocess::setAllNonBlocking() {
   for (auto& p : pipes_) {
-    int fd = p.parentFd;
+    int fd = p.pipe.fd();
     int flags = ::fcntl(fd, F_GETFL);
     checkUnixError(flags, "fcntl");
     int r = ::fcntl(fd, F_SETFL, flags | O_NONBLOCK);
@@ -244,12 +246,8 @@ void Subprocess::spawn(
   // Make a copy, we'll mutate options
   Options options(optionsIn);
 
-  // On error, close all of the pipes_
-  auto pipesGuard = makeGuard([&] {
-    for (auto& p : this->pipes_) {
-      CHECK_ERR(::close(p.parentFd));
-    }
-  });
+  // On error, close all pipes_ (ignoring errors, but that seems fine here).
+  auto pipesGuard = makeGuard([this] { pipes_.clear(); });
 
   // Create a pipe to use to receive error information from the child,
   // in case it fails before calling exec()
@@ -325,6 +323,9 @@ void Subprocess::spawnInternal(
       // doesn't need to reset the flag on its end, as we always dup2() the fd,
       // and dup2() fds don't share the close-on-exec flag.
 #if FOLLY_HAVE_PIPE2
+      // If possible, set close-on-exec atomically. Otherwise, a concurrent
+      // Subprocess invocation can fork() between "pipe" and "fnctl",
+      // causing FDs to leak.
       r = ::pipe2(fds, O_CLOEXEC);
       checkUnixError(r, "pipe2");
 #else
@@ -335,21 +336,21 @@ void Subprocess::spawnInternal(
       r = fcntl(fds[1], F_SETFD, FD_CLOEXEC);
       checkUnixError(r, "set FD_CLOEXEC");
 #endif
-      PipeInfo pinfo;
-      pinfo.direction = p.second;
+      pipes_.emplace_back();
+      Pipe& pipe = pipes_.back();
+      pipe.direction = p.second;
       int cfd;
       if (p.second == PIPE_IN) {
         // Child gets reading end
-        pinfo.parentFd = fds[1];
+        pipe.pipe = folly::File(fds[1], /*owns_fd=*/ true);
         cfd = fds[0];
       } else {
-        pinfo.parentFd = fds[0];
+        pipe.pipe = folly::File(fds[0], /*owns_fd=*/ true);
         cfd = fds[1];
       }
       p.second = cfd;  // ensure it gets dup2()ed
-      pinfo.childFd = p.first;
+      pipe.childFd = p.first;
       childFds.push_back(cfd);
-      pipes_.push_back(pinfo);
     }
   }
 
@@ -486,6 +487,12 @@ int Subprocess::prepareChild(const Options& options,
   }
 #endif
 
+  if (options.processGroupLeader_) {
+    if (setpgrp() == -1) {
+      return errno;
+    }
+  }
+
   return 0;
 }
 
@@ -493,7 +500,6 @@ int Subprocess::runChild(const char* executable,
                          char** argv, char** env,
                          const Options& options) const {
   // Now, finally, exec.
-  int r;
   if (options.usePath_) {
     ::execvp(executable, argv);
   } else {
@@ -538,6 +544,8 @@ ProcessReturnCode Subprocess::poll() {
   pid_t found = ::waitpid(pid_, &status, WNOHANG);
   checkUnixError(found, "waitpid");
   if (found != 0) {
+    // Though the child process had quit, this call does not close the pipes
+    // since its descendants may still be using them.
     returnCode_ = ProcessReturnCode(status);
     pid_ = -1;
   }
@@ -561,6 +569,8 @@ ProcessReturnCode Subprocess::wait() {
     found = ::waitpid(pid_, &status, 0);
   } while (found == -1 && errno == EINTR);
   checkUnixError(found, "waitpid");
+  // Though the child process had quit, this call does not close the pipes
+  // since its descendants may still be using them.
   DCHECK_EQ(found, pid_);
   returnCode_ = ProcessReturnCode(status);
   pid_ = -1;
@@ -704,12 +714,14 @@ std::pair<IOBufQueue, IOBufQueue> Subprocess::communicateIOBuf(
 
 void Subprocess::communicate(FdCallback readCallback,
                              FdCallback writeCallback) {
+  // This serves to prevent wait() followed by communicate(), but if you
+  // legitimately need that, send a patch to delete this line.
   returnCode_.enforce(ProcessReturnCode::RUNNING);
   setAllNonBlocking();
 
   std::vector<pollfd> fds;
   fds.reserve(pipes_.size());
-  std::vector<int> toClose;
+  std::vector<size_t> toClose;  // indexes into pipes_
   toClose.reserve(pipes_.size());
 
   while (!pipes_.empty()) {
@@ -718,7 +730,7 @@ void Subprocess::communicate(FdCallback readCallback,
 
     for (auto& p : pipes_) {
       pollfd pfd;
-      pfd.fd = p.parentFd;
+      pfd.fd = p.pipe.fd();
       // Yes, backwards, PIPE_IN / PIPE_OUT are defined from the
       // child's point of view.
       if (!p.enabled) {
@@ -739,15 +751,16 @@ void Subprocess::communicate(FdCallback readCallback,
     } while (r == -1 && errno == EINTR);
     checkUnixError(r, "poll");
 
-    for (int i = 0; i < pipes_.size(); ++i) {
+    for (size_t i = 0; i < pipes_.size(); ++i) {
       auto& p = pipes_[i];
-      DCHECK_EQ(fds[i].fd, p.parentFd);
+      auto parentFd = p.pipe.fd();
+      DCHECK_EQ(fds[i].fd, parentFd);
       short events = fds[i].revents;
 
       bool closed = false;
       if (events & POLLOUT) {
         DCHECK(!(events & POLLIN));
-        if (writeCallback(p.parentFd, p.childFd)) {
+        if (writeCallback(parentFd, p.childFd)) {
           toClose.push_back(i);
           closed = true;
         }
@@ -757,7 +770,7 @@ void Subprocess::communicate(FdCallback readCallback,
       // on) end of file
       if (events & (POLLIN | POLLHUP)) {
         DCHECK(!(events & POLLOUT));
-        if (readCallback(p.parentFd, p.childFd)) {
+        if (readCallback(parentFd, p.childFd)) {
           toClose.push_back(i);
           closed = true;
         }
@@ -772,7 +785,7 @@ void Subprocess::communicate(FdCallback readCallback,
     // Close the fds in reverse order so the indexes hold after erase()
     for (int idx : boost::adaptors::reverse(toClose)) {
       auto pos = pipes_.begin() + idx;
-      closeChecked(pos->parentFd);
+      pos->pipe.close();  // Throws on error
       pipes_.erase(pos);
     }
   }
@@ -786,10 +799,10 @@ bool Subprocess::notificationsEnabled(int childFd) const {
   return pipes_[findByChildFd(childFd)].enabled;
 }
 
-int Subprocess::findByChildFd(int childFd) const {
+size_t Subprocess::findByChildFd(int childFd) const {
   auto pos = std::lower_bound(
       pipes_.begin(), pipes_.end(), childFd,
-      [] (const PipeInfo& info, int fd) { return info.childFd < fd; });
+      [] (const Pipe& pipe, int fd) { return pipe.childFd < fd; });
   if (pos == pipes_.end() || pos->childFd != childFd) {
     throw std::invalid_argument(folly::to<std::string>(
         "child fd not found ", childFd));
@@ -799,10 +812,19 @@ int Subprocess::findByChildFd(int childFd) const {
 
 void Subprocess::closeParentFd(int childFd) {
   int idx = findByChildFd(childFd);
-  closeChecked(pipes_[idx].parentFd);
+  pipes_[idx].pipe.close();  // May throw
   pipes_.erase(pipes_.begin() + idx);
 }
 
+std::vector<Subprocess::ChildPipe> Subprocess::takeOwnershipOfPipes() {
+  std::vector<Subprocess::ChildPipe> pipes;
+  for (auto& p : pipes_) {
+    pipes.emplace_back(p.childFd, std::move(p.pipe));
+  }
+  pipes_.clear();
+  return pipes;
+}
+
 namespace {
 
 class Initializer {
@@ -818,4 +840,3 @@ Initializer initializer;
 }  // namespace
 
 }  // namespace folly
-