Mac: stop zombie ps processes from being created.

This fixes two bugs:
- the output buffer for the ps command being run could be too small (fix: buffer
  enlargened); and
- if the output buffer was too small, waitpid() wouldn't be called (fix: always
  call waitpid(), obviously).

BUG=31378
TEST=On a big, long-running Chrome/Chromium (with many processes, e.g., renderers), check that there are no zombie ps processes (with PPID the browser process); looking at about:memory may help speed up the creation of such zombies. Also, run the new test, ProcessUtilTest.GetAppOutputRestrictedNoZombies.

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

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

2 files changed, 34 insertions, 10 deletions

diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index 16469db..21626732 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -580,31 +580,33 @@ static bool GetAppOutputInternal(const CommandLine& cl, char* const envp[],
         close(pipe_fd[1]);
 
         char buffer[256];
-        std::string buf_output;
-        size_t output_left = max_output;
+        std::string output_buf;
+        size_t output_buf_left = max_output;
         ssize_t bytes_read = 1;  // A lie to properly handle |max_output == 0|
                                  // case in the logic below.
 
-        while (output_left > 0) {
+        while (output_buf_left > 0) {
           bytes_read = HANDLE_EINTR(read(pipe_fd[0], buffer,
-                                    std::min(output_left, sizeof(buffer))));
+                                    std::min(output_buf_left, sizeof(buffer))));
           if (bytes_read <= 0)
             break;
-          buf_output.append(buffer, bytes_read);
-          output_left -= static_cast<size_t>(bytes_read);
+          output_buf.append(buffer, bytes_read);
+          output_buf_left -= static_cast<size_t>(bytes_read);
         }
         close(pipe_fd[0]);
 
+        // Always wait for exit code (even if we know we'll declare success).
+        int exit_code = EXIT_FAILURE;
+        bool success = WaitForExitCode(pid, &exit_code);
+
         // If we stopped because we read as much as we wanted, we always declare
         // success (because the child may exit due to |SIGPIPE|).
-        if (output_left || bytes_read <= 0) {
-          int exit_code = EXIT_FAILURE;
-          bool success = WaitForExitCode(pid, &exit_code);
+        if (output_buf_left || bytes_read <= 0) {
           if (!success || exit_code != EXIT_SUCCESS)
             return false;
         }
 
-        output->swap(buf_output);
+        output->swap(output_buf);
         return true;
       }
   }
diff --git a/base/process_util_unittest.cc b/base/process_util_unittest.cc
index 8bbd784..4b313a3 100644
--- a/base/process_util_unittest.cc
+++ b/base/process_util_unittest.cc
@@ -337,6 +337,28 @@ TEST_F(ProcessUtilTest, GetAppOutputRestricted) {
   EXPECT_STREQ("", output.c_str());
 }
 
+TEST_F(ProcessUtilTest, GetAppOutputRestrictedNoZombies) {
+  std::vector<std::string> argv;
+  argv.push_back("/bin/sh");  // argv[0]
+  argv.push_back("-c");       // argv[1]
+  argv.push_back("echo 123456789012345678901234567890");  // argv[2]
+
+  // Run |GetAppOutputRestricted()| 300 (> default per-user processes on Mac OS
+  // 10.5) times with an output buffer big enough to capture all output.
+  for (int i = 0; i < 300; i++) {
+    std::string output;
+    EXPECT_TRUE(GetAppOutputRestricted(CommandLine(argv), &output, 100));
+    EXPECT_STREQ("123456789012345678901234567890\n", output.c_str());
+  }
+
+  // Ditto, but with an output buffer too small to capture all output.
+  for (int i = 0; i < 300; i++) {
+    std::string output;
+    EXPECT_TRUE(GetAppOutputRestricted(CommandLine(argv), &output, 10));
+    EXPECT_STREQ("1234567890", output.c_str());
+  }
+}
+
 #if defined(OS_LINUX)
 TEST_F(ProcessUtilTest, GetParentProcessId) {
   base::ProcessId ppid = GetParentProcessId(GetCurrentProcId());