Update Subprocess to throw if exec() fails

[folly.git] / folly / test / SubprocessTest.cpp

diff --git a/folly/test/SubprocessTest.cpp b/folly/test/SubprocessTest.cpp
index 17c145d177e93d913d311edae0a3996651cd6a7b..112e9aae1c45504ec24b9e12ef36d1b40841292e 100644 (file)
--- a/folly/test/SubprocessTest.cpp
+++ b/folly/test/SubprocessTest.cpp
@@ -17,10 +17,14 @@
 #include "folly/Subprocess.h"
 
 #include <unistd.h>
+#include <sys/types.h>
+#include <dirent.h>
 
+#include <boost/container/flat_set.hpp>
 #include <glog/logging.h>
 #include <gtest/gtest.h>
 
+#include "folly/Exception.h"
 #include "folly/Format.h"
 #include "folly/experimental/Gen.h"
 #include "folly/experimental/FileGen.h"
@@ -49,9 +53,27 @@ TEST(SimpleSubprocessTest, ExitsWithErrorChecked) {
   EXPECT_THROW(proc.waitChecked(), CalledProcessError);
 }
 
+#define EXPECT_SPAWN_ERROR(err, errMsg, cmd, ...) \
+  do { \
+    try { \
+      Subprocess proc(std::vector<std::string>{ (cmd), ## __VA_ARGS__ }); \
+      ADD_FAILURE() << "expected an error when running " << (cmd); \
+    } catch (const SubprocessSpawnError& ex) { \
+      EXPECT_EQ((err), ex.errnoValue()); \
+      if (StringPiece(ex.what()).find(errMsg) == StringPiece::npos) { \
+        ADD_FAILURE() << "failed to find \"" << (errMsg) << \
+          "\" in exception: \"" << ex.what() << "\""; \
+      } \
+    } \
+  } while (0)
+
 TEST(SimpleSubprocessTest, ExecFails) {
-  Subprocess proc(std::vector<std::string>{ "/no/such/file" });
-  EXPECT_EQ(127, proc.wait().exitStatus());
+  EXPECT_SPAWN_ERROR(ENOENT, "failed to execute /no/such/file:",
+                     "/no/such/file");
+  EXPECT_SPAWN_ERROR(EACCES, "failed to execute /etc/passwd:",
+                     "/etc/passwd");
+  EXPECT_SPAWN_ERROR(ENOTDIR, "failed to execute /etc/passwd/not/a/file:",
+                     "/etc/passwd/not/a/file");
 }
 
 TEST(SimpleSubprocessTest, ShellExitsSuccesssfully) {
@@ -64,6 +86,69 @@ TEST(SimpleSubprocessTest, ShellExitsWithError) {
   EXPECT_EQ(1, proc.wait().exitStatus());
 }
 
+namespace {
+boost::container::flat_set<int> getOpenFds() {
+  auto pid = getpid();
+  auto dirname = to<std::string>("/proc/", pid, "/fd");
+
+  boost::container::flat_set<int> fds;
+  for (fs::directory_iterator it(dirname);
+       it != fs::directory_iterator();
+       ++it) {
+    int fd = to<int>(it->path().filename().native());
+    fds.insert(fd);
+  }
+  return fds;
+}
+
+template<class Runnable>
+void checkFdLeak(const Runnable& r) {
+  // Get the currently open fds.  Check that they are the same both before and
+  // after calling the specified function.  We read the open fds from /proc.
+  // (If we wanted to work even on systems that don't have /proc, we could
+  // perhaps create and immediately close a socket both before and after
+  // running the function, and make sure we got the same fd number both times.)
+  auto fdsBefore = getOpenFds();
+  r();
+  auto fdsAfter = getOpenFds();
+  EXPECT_EQ(fdsAfter.size(), fdsBefore.size());
+}
+}
+
+// Make sure Subprocess doesn't leak any file descriptors
+TEST(SimpleSubprocessTest, FdLeakTest) {
+  // Normal execution
+  checkFdLeak([] {
+    Subprocess proc("true");
+    EXPECT_EQ(0, proc.wait().exitStatus());
+  });
+  // Normal execution with pipes
+  checkFdLeak([] {
+    Subprocess proc("echo foo; echo bar >&2",
+                    Subprocess::pipeStdout() | Subprocess::pipeStderr());
+    auto p = proc.communicate(Subprocess::readStdout() |
+                              Subprocess::readStderr());
+    EXPECT_EQ("foo\n", p.first);
+    EXPECT_EQ("bar\n", p.second);
+    proc.waitChecked();
+  });
+
+  // Test where the exec call fails()
+  checkFdLeak([] {
+    EXPECT_SPAWN_ERROR(ENOENT, "failed to execute", "/no/such/file");
+  });
+  // Test where the exec call fails() with pipes
+  checkFdLeak([] {
+    try {
+      Subprocess proc(std::vector<std::string>({"/no/such/file"}),
+                      Subprocess::pipeStdout().stderr(Subprocess::PIPE));
+      ADD_FAILURE() << "expected an error when running /no/such/file";
+    } catch (const SubprocessSpawnError& ex) {
+      EXPECT_EQ(ENOENT, ex.errnoValue());
+    }
+  });
+}
+
 TEST(ParentDeathSubprocessTest, ParentDeathSignal) {
   // Find out where we are.
   static constexpr size_t pathLength = 2048;
@@ -144,4 +229,3 @@ TEST(CommunicateSubprocessTest, Duplex) {
   EXPECT_EQ(std::string::npos, p.first.find_first_not_of('X'));
   proc.waitChecked();
 }
-