From 8bf9e83eaf0cac43462cb87dc3dc5a6ad2b58570 Mon Sep 17 00:00:00 2001
From: Jordan DeLong <jdelong@fb.com>
Date: Sun, 12 May 2013 14:33:28 -0700
Subject: [PATCH] Delay registration of SIGCHLD handler until after lwp fork

I think the cause of our issues with random failed-to-exec
errors is this SIGCHLD handler.  When the parent process asks a light
process to do a waitpid operation, we now can get EINTR for two
reasons.  It used to use signals only for implementing timeouts with
SIGALRM, but now it appears possible due to the SIGCHLD.  This could
lead us to report the waitpid as failing---which it looks can have
ExecFuture thinking running==false and reading the exit code as -1.

Also, now that we have a SIGCHLD handler (are we sure we want to keep
this?), using waitpid() without an EINTR loop is probably similarly
broken in the parent process.  (Also, isn't it basically incorrect to
use wait functions without an EINTR loop in general, though?)  I
didn't add an EINTR loop in do_waitpid in this diff because it's using
signals for timeout (it really should be checking a volatile
sigatomic_t that gets set by the handler, though)

Also, this means using Process::Exec is suspect in general.  All uses
looked like debugger or tests only (and when hphpc runs hhvm in a
subprocess), so I didn't nuke them (yet).  I deleted some random
network.h dead code that uses it, though.
---
 hphp/util/light_process.cpp | 26 +++++++++++++-------------
 hphp/util/network.cpp       | 24 ------------------------
 hphp/util/network.h         |  5 -----
 3 files changed, 13 insertions(+), 42 deletions(-)

diff --git a/hphp/util/light_process.cpp b/hphp/util/light_process.cpp
index 17319e17c..fdab1a800 100644
--- a/hphp/util/light_process.cpp
+++ b/hphp/util/light_process.cpp
@@ -319,18 +319,6 @@ void LightProcess::Initialize(const std::string &prefix, int count,
     return;
   }
 
-  if (!s_handlerInited) {
-    struct sigaction sa;
-    struct sigaction old_sa;
-    sa.sa_sigaction = &LightProcess::SigChldHandler;
-    sa.sa_flags = SA_SIGINFO | SA_NOCLDSTOP;
-    if (sigaction(SIGCHLD, &sa, &old_sa) != 0) {
-      Logger::Error("Couldn't install SIGCHLD handler");
-      abort();
-    }
-    s_handlerInited = true;
-  }
-
   g_procs.reset(new LightProcess[count]);
   g_procsCount = count;
 
@@ -344,6 +332,18 @@ void LightProcess::Initialize(const std::string &prefix, int count,
       break;
     }
   }
+
+  if (!s_handlerInited) {
+    struct sigaction sa;
+    struct sigaction old_sa;
+    sa.sa_sigaction = &LightProcess::SigChldHandler;
+    sa.sa_flags = SA_SIGINFO | SA_NOCLDSTOP;
+    if (sigaction(SIGCHLD, &sa, &old_sa) != 0) {
+      Logger::Error("Couldn't install SIGCHLD handler");
+      abort();
+    }
+    s_handlerInited = true;
+  }
 }
 
 bool LightProcess::initShadow(const std::string &prefix, int id,
@@ -503,7 +503,7 @@ void LightProcess::runShadow(int fdin, int fdout) {
   fclose(fout);
   ::close(m_afdt_fd);
   remove(m_afdtFilename.c_str());
-  exit(0);
+  _Exit(0);
 }
 
 int LightProcess::GetId() {
diff --git a/hphp/util/network.cpp b/hphp/util/network.cpp
index bee86f0b3..0ed475495 100644
--- a/hphp/util/network.cpp
+++ b/hphp/util/network.cpp
@@ -92,29 +92,5 @@ std::string Util::GetPrimaryIP() {
   return safe_inet_ntoa(in);
 }
 
-bool Util::GetNetworkStats(const char *iface, int &in_bps, int &out_bps) {
-  assert(iface && *iface);
-
-  const char *argv[] = {"", "1", "1", "-n", "DEV", nullptr};
-  string out;
-  Process::Exec("sar", argv, nullptr, out);
-
-  vector<string> lines;
-  Util::split('\n', out.c_str(), lines, true);
-  for (unsigned int i = 0; i < lines.size(); i++) {
-    string &line = lines[i];
-    if (line.find(iface) != string::npos) {
-      vector<string> fields;
-      Util::split(' ', line.c_str(), fields, true);
-      if (fields[1] == iface) {
-        in_bps = atoll(fields[4].c_str());
-        out_bps = atoll(fields[5].c_str());
-        return true;
-      }
-    }
-  }
-  return false;
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 }
diff --git a/hphp/util/network.h b/hphp/util/network.h
index 889398538..59bfdf32d 100644
--- a/hphp/util/network.h
+++ b/hphp/util/network.h
@@ -49,11 +49,6 @@ std::string safe_inet_ntoa(struct in_addr &in);
  */
 std::string GetPrimaryIP();
 
-/**
- * Get network bytes per second.
- */
-bool GetNetworkStats(const char *iface, int &in_bps, int &out_bps);
-
 ///////////////////////////////////////////////////////////////////////////////
 }}