diff options
| -rw-r--r-- | base/process_util_posix.cc | 22 | ||||
| -rw-r--r-- | base/process_util_unittest.cc | 22 | ||||
| -rw-r--r-- | chrome/browser/process_info_snapshot.h | 7 | ||||
| -rw-r--r-- | chrome/browser/process_info_snapshot_mac.cc | 16 |
4 files changed, 54 insertions, 13 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()); diff --git a/chrome/browser/process_info_snapshot.h b/chrome/browser/process_info_snapshot.h index 88064c6..14e8ceb 100644 --- a/chrome/browser/process_info_snapshot.h +++ b/chrome/browser/process_info_snapshot.h @@ -31,9 +31,14 @@ class ProcessInfoSnapshot { ProcessInfoSnapshot(); ~ProcessInfoSnapshot(); + // Maximum size of lists of PIDs which this class will accept; used in + // |Sample()| below. + static const size_t kMaxPidListSize; + // Capture a snapshot of process memory information (by running ps) for the // given list of PIDs. Call only from the file thread. - // |pid_list| - list of |ProcessId|s on which to capture information. + // |pid_list| - list of |ProcessId|s on which to capture information; must + // have no more than |kMaxPidListSize| (above) entries, // returns - |true| if okay, |false| on error. bool Sample(std::vector<base::ProcessId> pid_list); diff --git a/chrome/browser/process_info_snapshot_mac.cc b/chrome/browser/process_info_snapshot_mac.cc index 611f8f4..4e2608b 100644 --- a/chrome/browser/process_info_snapshot_mac.cc +++ b/chrome/browser/process_info_snapshot_mac.cc @@ -7,6 +7,7 @@ #include <iostream> #include <sstream> +#include "base/logging.h" #include "base/string_util.h" #include "base/thread.h" @@ -21,6 +22,8 @@ ProcessInfoSnapshot::~ProcessInfoSnapshot() { Reset(); } +const size_t ProcessInfoSnapshot::kMaxPidListSize = 1000; + // Capture the information by calling '/bin/ps'. // Note: we ignore the "tsiz" (text size) display option of ps because it's // always zero (tested on 10.5 and 10.6). @@ -28,6 +31,15 @@ bool ProcessInfoSnapshot::Sample(std::vector<base::ProcessId> pid_list) { const char* kPsPathName = "/bin/ps"; Reset(); + // Nothing to do if no PIDs given. + if (pid_list.size() == 0) + return true; + if (pid_list.size() > kMaxPidListSize) { + // The spec says |pid_list| *must* not have more than this many entries. + NOTREACHED(); + return false; + } + std::vector<std::string> argv; argv.push_back(kPsPathName); // Get PID, PPID, (real) UID, effective UID, resident set size, virtual memory @@ -43,8 +55,8 @@ bool ProcessInfoSnapshot::Sample(std::vector<base::ProcessId> pid_list) { std::string output; CommandLine command_line(argv); - if (!base::GetAppOutputRestricted(command_line, - &output, (pid_list.size() + 10) * 100)) { + // Limit output read to a megabyte for safety. + if (!base::GetAppOutputRestricted(command_line, &output, 1024 * 1024)) { LOG(ERROR) << "Failure running " << kPsPathName << " to acquire data."; return false; } |