Clean up orphaned testserver processes before launching a new one in net::TestServer

Several chrome tests have failed in recent days because some test cases crashed due to failures, leaving behind orphaned testserver processes. These test suites do not use out_of_proc_test_runner, and therefore, do not get the benefit of using the LaunchAppInNewProcessGroup()/KillProcessGroup() mechanism.

This patch implements an added layer of safety, by causing TestServer::LaunchPython() to first kill any remaining orphaned instances of testserver.py before launching a new one. The check for an orphaned testserver process on POSIX is a process with exe_name "python", a parent_pid of "1" (indicating that it's an orphan), and command line parameters that contain the strings "testserver.py" and "<port>" where <port> is the port that is being used by the test server instance.

BUG=55808,57253
TEST=Run a test that spawns a testserver and dies. Run another test. It shouldn't fail.

Review URL: http://codereview.chromium.org/3537002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@61584 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 56 insertions, 10 deletions

diff --git a/base/process_util.h b/base/process_util.h
index c513f3d..6876e88 100644
--- a/base/process_util.h
+++ b/base/process_util.h
@@ -67,10 +67,15 @@ struct ProcessEntry {
   ProcessId ppid_;
   ProcessId gid_;
   std::string exe_file_;
+  std::vector<std::string> cmd_line_args_;
 
   ProcessId pid() const { return pid_; }
   ProcessId parent_pid() const { return ppid_; }
+  ProcessId gid() const { return gid_; }
   const char* exe_file() const { return exe_file_.c_str(); }
+  const std::vector<std::string>& cmd_line_args() const {
+    return cmd_line_args_;
+  }
 };
 
 struct IoCounters {
diff --git a/base/process_util_linux.cc b/base/process_util_linux.cc
index 43e1a5c..4c034c4 100644
--- a/base/process_util_linux.cc
+++ b/base/process_util_linux.cc
@@ -31,15 +31,34 @@ enum ParsingState {
 };
 
 // Reads /proc/<pid>/stat and populates |proc_stats| with the values split by
-// spaces.
-void GetProcStats(pid_t pid, std::vector<std::string>* proc_stats) {
+// spaces. Returns true if successful.
+bool GetProcStats(pid_t pid, std::vector<std::string>* proc_stats) {
   FilePath stat_file("/proc");
   stat_file = stat_file.Append(base::IntToString(pid));
   stat_file = stat_file.Append("stat");
   std::string mem_stats;
   if (!file_util::ReadFileToString(stat_file, &mem_stats))
-    return;
+    return false;
   SplitString(mem_stats, ' ', proc_stats);
+  return true;
+}
+
+// Reads /proc/<pid>/cmdline and populates |proc_cmd_line_args| with the command
+// line arguments. Returns true if successful.
+// Note: /proc/<pid>/cmdline contains command line arguments separated by single
+// null characters. We tokenize it into a vector of strings using '\0' as a
+// delimiter.
+bool GetProcCmdline(pid_t pid, std::vector<std::string>* proc_cmd_line_args) {
+  FilePath cmd_line_file("/proc");
+  cmd_line_file = cmd_line_file.Append(base::IntToString(pid));
+  cmd_line_file = cmd_line_file.Append("cmdline");
+  std::string cmd_line;
+  if (!file_util::ReadFileToString(cmd_line_file, &cmd_line))
+    return false;
+  std::string delimiters;
+  delimiters.push_back('\0');
+  Tokenize(cmd_line, delimiters, proc_cmd_line_args);
+  return true;
 }
 
 }  // namespace
@@ -109,6 +128,7 @@ bool ProcessIterator::CheckForNextProcess() {
   dirent* slot = 0;
   const char* openparen;
   const char* closeparen;
+  std::vector<std::string> cmd_line_args;
 
   // Arbitrarily guess that there will never be more than 200 non-process
   // files in /proc.  Hardy has 53.
@@ -134,6 +154,12 @@ bool ProcessIterator::CheckForNextProcess() {
       continue;
     }
 
+    // Read the process's command line.
+    std::string pid_string(slot->d_name);
+    int pid;
+    if (StringToInt(pid_string, &pid) && !GetProcCmdline(pid, &cmd_line_args))
+      return false;
+
     // Read the process's status.
     char buf[NAME_MAX + 12];
     sprintf(buf, "/proc/%s/stat", slot->d_name);
@@ -175,6 +201,8 @@ bool ProcessIterator::CheckForNextProcess() {
   entry_.ppid_ = atoi(closeparen + 3);
   entry_.gid_ = atoi(strchr(closeparen + 4, ' '));
 
+  entry_.cmd_line_args_.assign(cmd_line_args.begin(), cmd_line_args.end());
+
   // TODO(port): read pid's commandline's $0, like killall does.  Using the
   // short name between openparen and closeparen won't work for long names!
   int len = closeparen - openparen - 1;
@@ -206,7 +234,8 @@ ProcessMetrics* ProcessMetrics::CreateProcessMetrics(ProcessHandle process) {
 // On linux, we return vsize.
 size_t ProcessMetrics::GetPagefileUsage() const {
   std::vector<std::string> proc_stats;
-  GetProcStats(process_, &proc_stats);
+  if (!GetProcStats(process_, &proc_stats))
+    LOG(WARNING) << "Failed to get process stats.";
   const size_t kVmSize = 22;
   if (proc_stats.size() > kVmSize) {
     int vm_size;
@@ -219,7 +248,8 @@ size_t ProcessMetrics::GetPagefileUsage() const {
 // On linux, we return the high water mark of vsize.
 size_t ProcessMetrics::GetPeakPagefileUsage() const {
   std::vector<std::string> proc_stats;
-  GetProcStats(process_, &proc_stats);
+  if (!GetProcStats(process_, &proc_stats))
+    LOG(WARNING) << "Failed to get process stats.";
   const size_t kVmPeak = 21;
   if (proc_stats.size() > kVmPeak) {
     int vm_peak;
@@ -232,7 +262,8 @@ size_t ProcessMetrics::GetPeakPagefileUsage() const {
 // On linux, we return RSS.
 size_t ProcessMetrics::GetWorkingSetSize() const {
   std::vector<std::string> proc_stats;
-  GetProcStats(process_, &proc_stats);
+  if (!GetProcStats(process_, &proc_stats))
+    LOG(WARNING) << "Failed to get process stats.";
   const size_t kVmRss = 23;
   if (proc_stats.size() > kVmRss) {
     int num_pages;
@@ -245,7 +276,8 @@ size_t ProcessMetrics::GetWorkingSetSize() const {
 // On linux, we return the high water mark of RSS.
 size_t ProcessMetrics::GetPeakWorkingSetSize() const {
   std::vector<std::string> proc_stats;
-  GetProcStats(process_, &proc_stats);
+  if (!GetProcStats(process_, &proc_stats))
+    LOG(WARNING) << "Failed to get process stats.";
   const size_t kVmHwm = 23;
   if (proc_stats.size() > kVmHwm) {
     int num_pages;
diff --git a/base/process_util_mac.mm b/base/process_util_mac.mm
index 2ea59a0..0865360 100644
--- a/base/process_util_mac.mm
+++ b/base/process_util_mac.mm
@@ -129,14 +129,23 @@ bool ProcessIterator::CheckForNextProcess() {
       continue;
     }
 
-    // Data starts w/ the full path null termed, so we have to extract just the
-    // executable name from the path.
-
+    // |data| contains all the command line parameters of the process, separated
+    // by blocks of one or more null characters. We tokenize |data| into a
+    // vector of strings using '\0' as a delimiter and populate
+    // |entry_.cmd_line_args_|.
+    std::string delimiters;
+    delimiters.push_back('\0');
+    Tokenize(data, delimiters, &entry_.cmd_line_args_);
+
+    // |data| starts with the full executable path followed by a null character.
+    // We search for the first instance of '\0' and extract everything before it
+    // to populate |entry_.exe_file_|.
     size_t exec_name_end = data.find('\0');
     if (exec_name_end == std::string::npos) {
       LOG(ERROR) << "command line data didn't match expected format";
       continue;
     }
+
     entry_.pid_ = kinfo.kp_proc.p_pid;
     entry_.ppid_ = kinfo.kp_eproc.e_ppid;
     entry_.gid_ = kinfo.kp_eproc.e_pgid;