diff options
21 files changed, 403 insertions, 501 deletions
diff --git a/base/process_util.h b/base/process_util.h index 1143cd8..874e656 100644 --- a/base/process_util.h +++ b/base/process_util.h @@ -499,6 +499,28 @@ BASE_EXPORT bool CleanupProcesses(const FilePath::StringType& executable_name, int exit_code, const ProcessFilter* filter); +// This method ensures that the specified process eventually terminates, and +// then it closes the given process handle. +// +// It assumes that the process has already been signalled to exit, and it +// begins by waiting a small amount of time for it to exit. If the process +// does not appear to have exited, then this function starts to become +// aggressive about ensuring that the process terminates. +// +// On Linux this method does not block the calling thread. +// On OS X this method may block for up to 2 seconds. +// +// NOTE: The process handle must have been opened with the PROCESS_TERMINATE +// and SYNCHRONIZE permissions. +// +BASE_EXPORT void EnsureProcessTerminated(ProcessHandle process_handle); + +#if defined(OS_POSIX) && !defined(OS_MACOSX) +// The nicer version of EnsureProcessTerminated() that is patient and will +// wait for |process_handle| to finish and then reap it. +BASE_EXPORT void EnsureProcessGetsReaped(ProcessHandle process_handle); +#endif + // This class provides a way to iterate through a list of processes on the // current machine with a specified filter. // To use, create an instance and then call NextProcessEntry() until it returns diff --git a/base/process_util_mac.mm b/base/process_util_mac.mm index 2d8bde4..6b39a61 100644 --- a/base/process_util_mac.mm +++ b/base/process_util_mac.mm @@ -7,6 +7,7 @@ #import <Cocoa/Cocoa.h> #include <crt_externs.h> #include <dlfcn.h> +#include <errno.h> #include <mach/mach.h> #include <mach/mach_init.h> #include <mach/mach_vm.h> @@ -16,7 +17,9 @@ #include <mach-o/nlist.h> #include <malloc/malloc.h> #import <objc/runtime.h> +#include <signal.h> #include <spawn.h> +#include <sys/event.h> #include <sys/mman.h> #include <sys/sysctl.h> #include <sys/types.h> @@ -27,6 +30,7 @@ #include "base/debug/debugger.h" #include "base/eintr_wrapper.h" +#include "base/file_util.h" #include "base/hash_tables.h" #include "base/logging.h" #include "base/mac/mac_util.h" @@ -990,4 +994,159 @@ ProcessId GetParentProcessId(ProcessHandle process) { return info.kp_eproc.e_ppid; } +namespace { + +const int kWaitBeforeKillSeconds = 2; + +// Reap |child| process. This call blocks until completion. +void BlockingReap(pid_t child) { + const pid_t result = HANDLE_EINTR(waitpid(child, NULL, 0)); + if (result == -1) { + DPLOG(ERROR) << "waitpid(" << child << ", NULL, 0)"; + } +} + +// Waits for |timeout| seconds for the given |child| to exit and reap it. If +// the child doesn't exit within the time specified, kills it. +// +// This function takes two approaches: first, it tries to use kqueue to +// observe when the process exits. kevent can monitor a kqueue with a +// timeout, so this method is preferred to wait for a specified period of +// time. Once the kqueue indicates the process has exited, waitpid will reap +// the exited child. If the kqueue doesn't provide an exit event notification, +// before the timeout expires, or if the kqueue fails or misbehaves, the +// process will be mercilessly killed and reaped. +// +// A child process passed to this function may be in one of several states: +// running, terminated and not yet reaped, and (apparently, and unfortunately) +// terminated and already reaped. Normally, a process will at least have been +// asked to exit before this function is called, but this is not required. +// If a process is terminating and unreaped, there may be a window between the +// time that kqueue will no longer recognize it and when it becomes an actual +// zombie that a non-blocking (WNOHANG) waitpid can reap. This condition is +// detected when kqueue indicates that the process is not running and a +// non-blocking waitpid fails to reap the process but indicates that it is +// still running. In this event, a blocking attempt to reap the process +// collects the known-dying child, preventing zombies from congregating. +// +// In the event that the kqueue misbehaves entirely, as it might under a +// EMFILE condition ("too many open files", or out of file descriptors), this +// function will forcibly kill and reap the child without delay. This +// eliminates another potential zombie vector. (If you're out of file +// descriptors, you're probably deep into something else, but that doesn't +// mean that zombies be allowed to kick you while you're down.) +// +// The fact that this function seemingly can be called to wait on a child +// that's not only already terminated but already reaped is a bit of a +// problem: a reaped child's pid can be reclaimed and may refer to a distinct +// process in that case. The fact that this function can seemingly be called +// to wait on a process that's not even a child is also a problem: kqueue will +// work in that case, but waitpid won't, and killing a non-child might not be +// the best approach. +void WaitForChildToDie(pid_t child, int timeout) { + DCHECK(child > 0); + DCHECK(timeout > 0); + + // DON'T ADD ANY EARLY RETURNS TO THIS FUNCTION without ensuring that + // |child| has been reaped. Specifically, even if a kqueue, kevent, or other + // call fails, this function should fall back to the last resort of trying + // to kill and reap the process. Not observing this rule will resurrect + // zombies. + + int result; + + int kq = HANDLE_EINTR(kqueue()); + if (kq == -1) { + DPLOG(ERROR) << "kqueue()"; + } else { + file_util::ScopedFD auto_close_kq(&kq); + + struct kevent change = {0}; + EV_SET(&change, child, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL); + result = HANDLE_EINTR(kevent(kq, &change, 1, NULL, 0, NULL)); + + if (result == -1) { + if (errno != ESRCH) { + DPLOG(ERROR) << "kevent (setup " << child << ")"; + } else { + // At this point, one of the following has occurred: + // 1. The process has died but has not yet been reaped. + // 2. The process has died and has already been reaped. + // 3. The process is in the process of dying. It's no longer + // kqueueable, but it may not be waitable yet either. Mark calls + // this case the "zombie death race". + + result = HANDLE_EINTR(waitpid(child, NULL, WNOHANG)); + + if (result != 0) { + // A positive result indicates case 1. waitpid succeeded and reaped + // the child. A result of -1 indicates case 2. The child has already + // been reaped. In both of these cases, no further action is + // necessary. + return; + } + + // |result| is 0, indicating case 3. The process will be waitable in + // short order. Fall back out of the kqueue code to kill it (for good + // measure) and reap it. + } + } else { + // Keep track of the elapsed time to be able to restart kevent if it's + // interrupted. + TimeDelta remaining_delta = TimeDelta::FromSeconds(timeout); + Time deadline = Time::Now() + remaining_delta; + result = -1; + struct kevent event = {0}; + while (remaining_delta.InMilliseconds() > 0) { + const struct timespec remaining_timespec = remaining_delta.ToTimeSpec(); + result = kevent(kq, NULL, 0, &event, 1, &remaining_timespec); + if (result == -1 && errno == EINTR) { + remaining_delta = deadline - Time::Now(); + result = 0; + } else { + break; + } + } + + if (result == -1) { + DPLOG(ERROR) << "kevent (wait " << child << ")"; + } else if (result > 1) { + DLOG(ERROR) << "kevent (wait " << child << "): unexpected result " + << result; + } else if (result == 1) { + if ((event.fflags & NOTE_EXIT) && + (event.ident == static_cast<uintptr_t>(child))) { + // The process is dead or dying. This won't block for long, if at + // all. + BlockingReap(child); + return; + } else { + DLOG(ERROR) << "kevent (wait " << child + << "): unexpected event: fflags=" << event.fflags + << ", ident=" << event.ident; + } + } + } + } + + // The child is still alive, or is very freshly dead. Be sure by sending it + // a signal. This is safe even if it's freshly dead, because it will be a + // zombie (or on the way to zombiedom) and kill will return 0 even if the + // signal is not delivered to a live process. + result = kill(child, SIGKILL); + if (result == -1) { + DPLOG(ERROR) << "kill(" << child << ", SIGKILL)"; + } else { + // The child is definitely on the way out now. BlockingReap won't need to + // wait for long, if at all. + BlockingReap(child); + } +} + +} // namespace + +void EnsureProcessTerminated(ProcessHandle process) { + WaitForChildToDie(process, kWaitBeforeKillSeconds); +} + } // namespace base diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc index 79ce1f7..f0e9fb0 100644 --- a/base/process_util_posix.cc +++ b/base/process_util_posix.cc @@ -1210,4 +1210,101 @@ bool CleanupProcesses(const FilePath::StringType& executable_name, return exited_cleanly; } +#if !defined(OS_MACOSX) + +namespace { + +// Return true if the given child is dead. This will also reap the process. +// Doesn't block. +static bool IsChildDead(pid_t child) { + const pid_t result = HANDLE_EINTR(waitpid(child, NULL, WNOHANG)); + if (result == -1) { + DPLOG(ERROR) << "waitpid(" << child << ")"; + NOTREACHED(); + } else if (result > 0) { + // The child has died. + return true; + } + + return false; +} + +// A thread class which waits for the given child to exit and reaps it. +// If the child doesn't exit within a couple of seconds, kill it. +class BackgroundReaper : public PlatformThread::Delegate { + public: + BackgroundReaper(pid_t child, unsigned timeout) + : child_(child), + timeout_(timeout) { + } + + void ThreadMain() { + WaitForChildToDie(); + delete this; + } + + void WaitForChildToDie() { + // Wait forever case. + if (timeout_ == 0) { + pid_t r = HANDLE_EINTR(waitpid(child_, NULL, 0)); + if (r != child_) { + DPLOG(ERROR) << "While waiting for " << child_ + << " to terminate, we got the following result: " << r; + } + return; + } + + // There's no good way to wait for a specific child to exit in a timed + // fashion. (No kqueue on Linux), so we just loop and sleep. + + // Wait for 2 * timeout_ 500 milliseconds intervals. + for (unsigned i = 0; i < 2 * timeout_; ++i) { + PlatformThread::Sleep(500); // 0.5 seconds + if (IsChildDead(child_)) + return; + } + + if (kill(child_, SIGKILL) == 0) { + // SIGKILL is uncatchable. Since the signal was delivered, we can + // just wait for the process to die now in a blocking manner. + if (HANDLE_EINTR(waitpid(child_, NULL, 0)) < 0) + DPLOG(WARNING) << "waitpid"; + } else { + DLOG(ERROR) << "While waiting for " << child_ << " to terminate we" + << " failed to deliver a SIGKILL signal (" << errno << ")."; + } + } + + private: + const pid_t child_; + // Number of seconds to wait, if 0 then wait forever and do not attempt to + // kill |child_|. + const unsigned timeout_; + + DISALLOW_COPY_AND_ASSIGN(BackgroundReaper); +}; + +} // namespace + +void EnsureProcessTerminated(ProcessHandle process) { + // If the child is already dead, then there's nothing to do. + if (IsChildDead(process)) + return; + + const unsigned timeout = 2; // seconds + BackgroundReaper* reaper = new BackgroundReaper(process, timeout); + PlatformThread::CreateNonJoinable(0, reaper); +} + +void EnsureProcessGetsReaped(ProcessHandle process) { + // If the child is already dead, then there's nothing to do. + if (IsChildDead(process)) + return; + + BackgroundReaper* reaper = new BackgroundReaper(process, 0); + PlatformThread::CreateNonJoinable(0, reaper); +} + +#endif // !defined(OS_MACOSX) + } // namespace base diff --git a/base/process_util_unittest.cc b/base/process_util_unittest.cc index 452892e..1b0b114 100644 --- a/base/process_util_unittest.cc +++ b/base/process_util_unittest.cc @@ -833,6 +833,51 @@ TEST_F(ProcessUtilTest, ParseProcStatCPU) { } #endif // defined(OS_LINUX) || defined(OS_ANDROID) +// TODO(port): port those unit tests. +bool IsProcessDead(base::ProcessHandle child) { + // waitpid() will actually reap the process which is exactly NOT what we + // want to test for. The good thing is that if it can't find the process + // we'll get a nice value for errno which we can test for. + const pid_t result = HANDLE_EINTR(waitpid(child, NULL, WNOHANG)); + return result == -1 && errno == ECHILD; +} + +TEST_F(ProcessUtilTest, DelayedTermination) { + base::ProcessHandle child_process = + SpawnChild("process_util_test_never_die", false); + ASSERT_TRUE(child_process); + base::EnsureProcessTerminated(child_process); + base::WaitForSingleProcess(child_process, 5000); + + // Check that process was really killed. + EXPECT_TRUE(IsProcessDead(child_process)); + base::CloseProcessHandle(child_process); +} + +MULTIPROCESS_TEST_MAIN(process_util_test_never_die) { + while (1) { + sleep(500); + } + return 0; +} + +TEST_F(ProcessUtilTest, ImmediateTermination) { + base::ProcessHandle child_process = + SpawnChild("process_util_test_die_immediately", false); + ASSERT_TRUE(child_process); + // Give it time to die. + sleep(2); + base::EnsureProcessTerminated(child_process); + + // Check that process was really killed. + EXPECT_TRUE(IsProcessDead(child_process)); + base::CloseProcessHandle(child_process); +} + +MULTIPROCESS_TEST_MAIN(process_util_test_die_immediately) { + return 0; +} + #endif // defined(OS_POSIX) // TODO(vandebo) make this work on Windows too. diff --git a/base/process_util_win.cc b/base/process_util_win.cc index 85aeace..891dc3b 100644 --- a/base/process_util_win.cc +++ b/base/process_util_win.cc @@ -16,8 +16,10 @@ #include "base/debug/stack_trace.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" +#include "base/message_loop.h" #include "base/metrics/histogram.h" #include "base/sys_info.h" +#include "base/win/object_watcher.h" #include "base/win/scoped_handle.h" #include "base/win/windows_version.h" @@ -37,6 +39,9 @@ const DWORD kDebuggerInactiveExitCode = 0xC0000354; const DWORD kKeyboardInterruptExitCode = 0xC000013A; const DWORD kDebuggerTerminatedExitCode = 0x40010004; +// Maximum amount of time (in milliseconds) to wait for the process to exit. +static const int kWaitInterval = 2000; + // This exit code is used by the Windows task manager when it kills a // process. It's value is obviously not that unique, and it's // surprising to me that the task manager uses this value, but it @@ -103,6 +108,58 @@ void OnNoMemory() { _exit(1); } +class TimerExpiredTask : public Task, + public win::ObjectWatcher::Delegate { + public: + explicit TimerExpiredTask(ProcessHandle process) : process_(process) { + watcher_.StartWatching(process_, this); + } + + virtual ~TimerExpiredTask() { + if (process_) { + KillProcess(); + DCHECK(!process_) << "Make sure to close the handle."; + } + } + + // Task --------------------------------------------------------------------- + + virtual void Run() { + if (process_) + KillProcess(); + } + + // MessageLoop::Watcher ----------------------------------------------------- + + virtual void OnObjectSignaled(HANDLE object) { + // When we're called from KillProcess, the ObjectWatcher may still be + // watching. the process handle, so make sure it has stopped. + watcher_.StopWatching(); + + CloseHandle(process_); + process_ = NULL; + } + + private: + void KillProcess() { + // OK, time to get frisky. We don't actually care when the process + // terminates. We just care that it eventually terminates, and that's what + // TerminateProcess should do for us. Don't check for the result code since + // it fails quite often. This should be investigated eventually. + base::KillProcess(process_, kProcessKilledExitCode, false); + + // Now, just cleanup as if the process exited normally. + OnObjectSignaled(process_); + } + + // The process that we are watching. + ProcessHandle process_; + + win::ObjectWatcher watcher_; + + DISALLOW_COPY_AND_ASSIGN(TimerExpiredTask); +}; + } // namespace ProcessId GetCurrentProcId() { @@ -551,6 +608,20 @@ bool CleanupProcesses(const std::wstring& executable_name, return exited_cleanly; } +void EnsureProcessTerminated(ProcessHandle process) { + DCHECK(process != GetCurrentProcess()); + + // If already signaled, then we are done! + if (WaitForSingleObject(process, 0) == WAIT_OBJECT_0) { + CloseHandle(process); + return; + } + + MessageLoop::current()->PostDelayedTask(FROM_HERE, + new TimerExpiredTask(process), + kWaitInterval); +} + /////////////////////////////////////////////////////////////////////////////// // ProcesMetrics diff --git a/chrome/browser/platform_util_chromeos.cc b/chrome/browser/platform_util_chromeos.cc index 1494146..cf860c5 100644 --- a/chrome/browser/platform_util_chromeos.cc +++ b/chrome/browser/platform_util_chromeos.cc @@ -6,14 +6,12 @@ #include "base/bind.h" #include "base/file_util.h" -#include "base/process_util.h" #include "base/task.h" #include "base/utf_string_conversions.h" #include "chrome/browser/extensions/file_manager_util.h" #include "chrome/browser/tabs/tab_strip_model.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_list.h" -#include "content/common/process_watcher.h" #include "content/public/browser/browser_thread.h" #include "googleurl/src/gurl.h" diff --git a/chrome/browser/platform_util_linux.cc b/chrome/browser/platform_util_linux.cc index e0a0b94..1064a34 100644 --- a/chrome/browser/platform_util_linux.cc +++ b/chrome/browser/platform_util_linux.cc @@ -10,7 +10,6 @@ #include "base/file_util.h" #include "base/process_util.h" #include "base/utf_string_conversions.h" -#include "content/common/process_watcher.h" #include "content/public/browser/browser_thread.h" #include "googleurl/src/gurl.h" @@ -43,7 +42,7 @@ void XDGUtil(const std::string& util, const std::string& arg) { base::LaunchOptions options; options.environ = &env; if (base::LaunchProcess(argv, options, &handle)) - ProcessWatcher::EnsureProcessGetsReaped(handle); + base::EnsureProcessGetsReaped(handle); } void XDGOpen(const std::string& path) { diff --git a/chrome/browser/printing/printer_manager_dialog_linux.cc b/chrome/browser/printing/printer_manager_dialog_linux.cc index 967f30e..410d849 100644 --- a/chrome/browser/printing/printer_manager_dialog_linux.cc +++ b/chrome/browser/printing/printer_manager_dialog_linux.cc @@ -8,7 +8,6 @@ #include "base/environment.h" #include "base/nix/xdg_util.h" #include "base/process_util.h" -#include "content/common/process_watcher.h" #include "content/public/browser/browser_thread.h" using base::Environment; @@ -54,7 +53,7 @@ void DetectAndOpenPrinterConfigDialog() { LOG(ERROR) << "Failed to open printer manager dialog "; return; } - ProcessWatcher::EnsureProcessGetsReaped(handle); + base::EnsureProcessGetsReaped(handle); } } // anonymous namespace diff --git a/chrome/browser/ui/webui/options/advanced_options_utils_x11.cc b/chrome/browser/ui/webui/options/advanced_options_utils_x11.cc index fd07b1a..7b68ba3 100644 --- a/chrome/browser/ui/webui/options/advanced_options_utils_x11.cc +++ b/chrome/browser/ui/webui/options/advanced_options_utils_x11.cc @@ -14,7 +14,6 @@ #include "base/process_util.h" #include "base/string_util.h" #include "content/browser/tab_contents/tab_contents.h" -#include "content/common/process_watcher.h" #include "content/public/browser/browser_thread.h" using content::BrowserThread; @@ -84,7 +83,7 @@ bool StartProxyConfigUtil(TabContents* tab_contents, const char* command[]) { LOG(ERROR) << "StartProxyConfigUtil failed to start " << command[0]; return false; } - ProcessWatcher::EnsureProcessGetsReaped(handle); + base::EnsureProcessGetsReaped(handle); return true; } diff --git a/content/browser/browser_child_process_host.cc b/content/browser/browser_child_process_host.cc index 5327c1d..2268486 100644 --- a/content/browser/browser_child_process_host.cc +++ b/content/browser/browser_child_process_host.cc @@ -16,7 +16,6 @@ #include "content/browser/renderer_host/resource_message_filter.h" #include "content/browser/trace_message_filter.h" #include "content/common/plugin_messages.h" -#include "content/common/process_watcher.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/content_browser_client.h" #include "content/public/browser/notification_service.h" diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc index c3df910..a9fdfe7 100644 --- a/content/browser/child_process_launcher.cc +++ b/content/browser/child_process_launcher.cc @@ -11,10 +11,10 @@ #include "base/file_util.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" +#include "base/process_util.h" #include "base/synchronization/lock.h" #include "base/threading/thread.h" #include "content/common/chrome_descriptors.h" -#include "content/common/process_watcher.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/content_browser_client.h" #include "content/public/common/content_switches.h" @@ -280,7 +280,7 @@ class ChildProcessLauncher::Context } else #endif // !OS_MACOSX { - ProcessWatcher::EnsureProcessTerminated(handle); + base::EnsureProcessTerminated(handle); } #endif // OS_POSIX process.Close(); diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc index de637bd..2e0fe9b 100644 --- a/content/browser/renderer_host/render_process_host_impl.cc +++ b/content/browser/renderer_host/render_process_host_impl.cc @@ -26,6 +26,7 @@ #include "base/metrics/histogram.h" #include "base/path_service.h" #include "base/platform_file.h" +#include "base/process_util.h" #include "base/rand_util.h" #include "base/stl_util.h" #include "base/string_util.h" @@ -78,7 +79,6 @@ #include "content/common/child_process_messages.h" #include "content/common/gpu/gpu_messages.h" #include "content/public/browser/notification_service.h" -#include "content/common/process_watcher.h" #include "content/common/resource_messages.h" #include "content/common/view_messages.h" #include "content/public/browser/content_browser_client.h" diff --git a/content/browser/zygote_host_linux.cc b/content/browser/zygote_host_linux.cc index 19dddd2..a7671e3 100644 --- a/content/browser/zygote_host_linux.cc +++ b/content/browser/zygote_host_linux.cc @@ -26,7 +26,6 @@ #include "base/time.h" #include "base/utf_string_conversions.h" #include "content/browser/renderer_host/render_sandbox_host_linux.h" -#include "content/common/process_watcher.h" #include "content/common/unix_domain_socket_posix.h" #include "content/public/browser/content_browser_client.h" #include "content/public/common/content_switches.h" @@ -203,7 +202,7 @@ void ZygoteHost::Init(const std::string& sandbox_cmd) { if (process != pid_) { // Reap the sandbox. - ProcessWatcher::EnsureProcessGetsReaped(process); + base::EnsureProcessGetsReaped(process); } } else { // Not using the SUID sandbox. @@ -379,7 +378,7 @@ void ZygoteHost::AdjustRendererOOMScore(base::ProcessHandle pid, int score) { base::ProcessHandle sandbox_helper_process; if (base::LaunchProcess(adj_oom_score_cmdline, base::LaunchOptions(), &sandbox_helper_process)) { - ProcessWatcher::EnsureProcessGetsReaped(sandbox_helper_process); + base::EnsureProcessGetsReaped(sandbox_helper_process); } } else if (!using_suid_sandbox_) { if (!base::AdjustOOMScore(pid, score)) diff --git a/content/browser/zygote_main_linux.cc b/content/browser/zygote_main_linux.cc index e2e174f..472b0b8 100644 --- a/content/browser/zygote_main_linux.cc +++ b/content/browser/zygote_main_linux.cc @@ -31,7 +31,6 @@ #include "content/common/chrome_descriptors.h" #include "content/common/font_config_ipc_linux.h" #include "content/common/pepper_plugin_registry.h" -#include "content/common/process_watcher.h" #include "content/common/sandbox_methods_linux.h" #include "content/common/seccomp_sandbox.h" #include "content/common/set_process_title.h" @@ -220,7 +219,7 @@ class Zygote { actual_child = child; } - ProcessWatcher::EnsureProcessTerminated(actual_child); + base::EnsureProcessTerminated(actual_child); } void HandleGetTerminationStatus(int fd, const Pickle& pickle, void* iter) { diff --git a/content/common/process_watcher.h b/content/common/process_watcher.h deleted file mode 100644 index f8ad6fbc..0000000 --- a/content/common/process_watcher.h +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CONTENT_COMMON_PROCESS_WATCHER_H_ -#define CONTENT_COMMON_PROCESS_WATCHER_H_ -#pragma once - -#include "build/build_config.h" - -#include "base/basictypes.h" -#include "base/process.h" -#include "content/common/content_export.h" - -class CONTENT_EXPORT ProcessWatcher { - public: - // This method ensures that the specified process eventually terminates, and - // then it closes the given process handle. - // - // It assumes that the process has already been signalled to exit, and it - // begins by waiting a small amount of time for it to exit. If the process - // does not appear to have exited, then this function starts to become - // aggressive about ensuring that the process terminates. - // - // On Linux this method does not block the calling thread. - // On OS X this method may block for up to 2 seconds. - // - // NOTE: The process handle must have been opened with the PROCESS_TERMINATE - // and SYNCHRONIZE permissions. - // - static void EnsureProcessTerminated(base::ProcessHandle process_handle); - -#if defined(OS_POSIX) && !defined(OS_MACOSX) - // The nicer version of EnsureProcessTerminated() that is patient and will - // wait for |process_handle| to finish and then reap it. - static void EnsureProcessGetsReaped(base::ProcessHandle process_handle); -#endif - - private: - // Do not instantiate this class. - ProcessWatcher(); - - DISALLOW_COPY_AND_ASSIGN(ProcessWatcher); -}; - -#endif // CONTENT_COMMON_PROCESS_WATCHER_H_ diff --git a/content/common/process_watcher_mac.cc b/content/common/process_watcher_mac.cc deleted file mode 100644 index 6af9ffc..0000000 --- a/content/common/process_watcher_mac.cc +++ /dev/null @@ -1,170 +0,0 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "content/common/process_watcher.h" - -#include <errno.h> -#include <signal.h> -#include <sys/event.h> -#include <sys/types.h> -#include <sys/wait.h> - -#include "base/eintr_wrapper.h" -#include "base/file_util.h" -#include "base/time.h" - -namespace { - -const int kWaitBeforeKillSeconds = 2; - -// Reap |child| process. This call blocks until completion. -void BlockingReap(pid_t child) { - const pid_t result = HANDLE_EINTR(waitpid(child, NULL, 0)); - if (result == -1) { - DPLOG(ERROR) << "waitpid(" << child << ", NULL, 0)"; - } -} - -// Waits for |timeout| seconds for the given |child| to exit and reap it. If -// the child doesn't exit within the time specified, kills it. -// -// This function takes two approaches: first, it tries to use kqueue to -// observe when the process exits. kevent can monitor a kqueue with a -// timeout, so this method is preferred to wait for a specified period of -// time. Once the kqueue indicates the process has exited, waitpid will reap -// the exited child. If the kqueue doesn't provide an exit event notification, -// before the timeout expires, or if the kqueue fails or misbehaves, the -// process will be mercilessly killed and reaped. -// -// A child process passed to this function may be in one of several states: -// running, terminated and not yet reaped, and (apparently, and unfortunately) -// terminated and already reaped. Normally, a process will at least have been -// asked to exit before this function is called, but this is not required. -// If a process is terminating and unreaped, there may be a window between the -// time that kqueue will no longer recognize it and when it becomes an actual -// zombie that a non-blocking (WNOHANG) waitpid can reap. This condition is -// detected when kqueue indicates that the process is not running and a -// non-blocking waitpid fails to reap the process but indicates that it is -// still running. In this event, a blocking attempt to reap the process -// collects the known-dying child, preventing zombies from congregating. -// -// In the event that the kqueue misbehaves entirely, as it might under a -// EMFILE condition ("too many open files", or out of file descriptors), this -// function will forcibly kill and reap the child without delay. This -// eliminates another potential zombie vector. (If you're out of file -// descriptors, you're probably deep into something else, but that doesn't -// mean that zombies be allowed to kick you while you're down.) -// -// The fact that this function seemingly can be called to wait on a child -// that's not only already terminated but already reaped is a bit of a -// problem: a reaped child's pid can be reclaimed and may refer to a distinct -// process in that case. The fact that this function can seemingly be called -// to wait on a process that's not even a child is also a problem: kqueue will -// work in that case, but waitpid won't, and killing a non-child might not be -// the best approach. -void WaitForChildToDie(pid_t child, int timeout) { - DCHECK(child > 0); - DCHECK(timeout > 0); - - // DON'T ADD ANY EARLY RETURNS TO THIS FUNCTION without ensuring that - // |child| has been reaped. Specifically, even if a kqueue, kevent, or other - // call fails, this function should fall back to the last resort of trying - // to kill and reap the process. Not observing this rule will resurrect - // zombies. - - int result; - - int kq = HANDLE_EINTR(kqueue()); - if (kq == -1) { - DPLOG(ERROR) << "kqueue()"; - } else { - file_util::ScopedFD auto_close_kq(&kq); - - struct kevent change = {0}; - EV_SET(&change, child, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL); - result = HANDLE_EINTR(kevent(kq, &change, 1, NULL, 0, NULL)); - - if (result == -1) { - if (errno != ESRCH) { - DPLOG(ERROR) << "kevent (setup " << child << ")"; - } else { - // At this point, one of the following has occurred: - // 1. The process has died but has not yet been reaped. - // 2. The process has died and has already been reaped. - // 3. The process is in the process of dying. It's no longer - // kqueueable, but it may not be waitable yet either. Mark calls - // this case the "zombie death race". - - result = HANDLE_EINTR(waitpid(child, NULL, WNOHANG)); - - if (result != 0) { - // A positive result indicates case 1. waitpid succeeded and reaped - // the child. A result of -1 indicates case 2. The child has already - // been reaped. In both of these cases, no further action is - // necessary. - return; - } - - // |result| is 0, indicating case 3. The process will be waitable in - // short order. Fall back out of the kqueue code to kill it (for good - // measure) and reap it. - } - } else { - // Keep track of the elapsed time to be able to restart kevent if it's - // interrupted. - base::TimeDelta remaining_delta = base::TimeDelta::FromSeconds(timeout); - base::Time deadline = base::Time::Now() + remaining_delta; - result = -1; - struct kevent event = {0}; - while (remaining_delta.InMilliseconds() > 0) { - const struct timespec remaining_timespec = remaining_delta.ToTimeSpec(); - result = kevent(kq, NULL, 0, &event, 1, &remaining_timespec); - if (result == -1 && errno == EINTR) { - remaining_delta = deadline - base::Time::Now(); - result = 0; - } else { - break; - } - } - - if (result == -1) { - DPLOG(ERROR) << "kevent (wait " << child << ")"; - } else if (result > 1) { - DLOG(ERROR) << "kevent (wait " << child << "): unexpected result " - << result; - } else if (result == 1) { - if ((event.fflags & NOTE_EXIT) && - (event.ident == static_cast<uintptr_t>(child))) { - // The process is dead or dying. This won't block for long, if at - // all. - BlockingReap(child); - return; - } else { - DLOG(ERROR) << "kevent (wait " << child - << "): unexpected event: fflags=" << event.fflags - << ", ident=" << event.ident; - } - } - } - } - - // The child is still alive, or is very freshly dead. Be sure by sending it - // a signal. This is safe even if it's freshly dead, because it will be a - // zombie (or on the way to zombiedom) and kill will return 0 even if the - // signal is not delivered to a live process. - result = kill(child, SIGKILL); - if (result == -1) { - DPLOG(ERROR) << "kill(" << child << ", SIGKILL)"; - } else { - // The child is definitely on the way out now. BlockingReap won't need to - // wait for long, if at all. - BlockingReap(child); - } -} - -} // namespace - -void ProcessWatcher::EnsureProcessTerminated(base::ProcessHandle process) { - WaitForChildToDie(process, kWaitBeforeKillSeconds); -} diff --git a/content/common/process_watcher_posix.cc b/content/common/process_watcher_posix.cc deleted file mode 100644 index f296106..0000000 --- a/content/common/process_watcher_posix.cc +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "content/common/process_watcher.h" - -#include <errno.h> -#include <signal.h> -#include <sys/types.h> -#include <sys/wait.h> - -#include "base/eintr_wrapper.h" -#include "base/logging.h" -#include "base/threading/platform_thread.h" - -// Return true if the given child is dead. This will also reap the process. -// Doesn't block. -static bool IsChildDead(pid_t child) { - const pid_t result = HANDLE_EINTR(waitpid(child, NULL, WNOHANG)); - if (result == -1) { - DPLOG(ERROR) << "waitpid(" << child << ")"; - NOTREACHED(); - } else if (result > 0) { - // The child has died. - return true; - } - - return false; -} - -// A thread class which waits for the given child to exit and reaps it. -// If the child doesn't exit within a couple of seconds, kill it. -class BackgroundReaper : public base::PlatformThread::Delegate { - public: - BackgroundReaper(pid_t child, unsigned timeout) - : child_(child), - timeout_(timeout) { - } - - void ThreadMain() { - WaitForChildToDie(); - delete this; - } - - void WaitForChildToDie() { - // Wait forever case. - if (timeout_ == 0) { - pid_t r = HANDLE_EINTR(waitpid(child_, NULL, 0)); - if (r != child_) { - DPLOG(ERROR) << "While waiting for " << child_ - << " to terminate, we got the following result: " << r; - } - return; - } - - // There's no good way to wait for a specific child to exit in a timed - // fashion. (No kqueue on Linux), so we just loop and sleep. - - // Wait for 2 * timeout_ 500 milliseconds intervals. - for (unsigned i = 0; i < 2 * timeout_; ++i) { - base::PlatformThread::Sleep(500); // 0.5 seconds - if (IsChildDead(child_)) - return; - } - - if (kill(child_, SIGKILL) == 0) { - // SIGKILL is uncatchable. Since the signal was delivered, we can - // just wait for the process to die now in a blocking manner. - if (HANDLE_EINTR(waitpid(child_, NULL, 0)) < 0) - DPLOG(WARNING) << "waitpid"; - } else { - DLOG(ERROR) << "While waiting for " << child_ << " to terminate we" - << " failed to deliver a SIGKILL signal (" << errno << ")."; - } - } - - private: - const pid_t child_; - // Number of seconds to wait, if 0 then wait forever and do not attempt to - // kill |child_|. - const unsigned timeout_; - - DISALLOW_COPY_AND_ASSIGN(BackgroundReaper); -}; - -// static -void ProcessWatcher::EnsureProcessTerminated(base::ProcessHandle process) { - // If the child is already dead, then there's nothing to do. - if (IsChildDead(process)) - return; - - const unsigned timeout = 2; // seconds - BackgroundReaper* reaper = new BackgroundReaper(process, timeout); - base::PlatformThread::CreateNonJoinable(0, reaper); -} - -// static -void ProcessWatcher::EnsureProcessGetsReaped(base::ProcessHandle process) { - // If the child is already dead, then there's nothing to do. - if (IsChildDead(process)) - return; - - BackgroundReaper* reaper = new BackgroundReaper(process, 0); - base::PlatformThread::CreateNonJoinable(0, reaper); -} diff --git a/content/common/process_watcher_unittest.cc b/content/common/process_watcher_unittest.cc deleted file mode 100644 index 139fd11..0000000 --- a/content/common/process_watcher_unittest.cc +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "content/common/process_watcher.h" - -#include <sys/wait.h> - -#include "base/eintr_wrapper.h" -#include "base/process_util.h" -#include "base/test/multiprocess_test.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "testing/multiprocess_func_list.h" - -class ProcessWatcherTest : public base::MultiProcessTest { -}; - -namespace { - -bool IsProcessDead(base::ProcessHandle child) { - // waitpid() will actually reap the process which is exactly NOT what we - // want to test for. The good thing is that if it can't find the process - // we'll get a nice value for errno which we can test for. - const pid_t result = HANDLE_EINTR(waitpid(child, NULL, WNOHANG)); - return result == -1 && errno == ECHILD; -} - -} // namespace - -TEST_F(ProcessWatcherTest, DelayedTermination) { - base::ProcessHandle child_process = - SpawnChild("process_watcher_test_never_die", false); - ASSERT_TRUE(child_process); - ProcessWatcher::EnsureProcessTerminated(child_process); - base::WaitForSingleProcess(child_process, 5000); - - // Check that process was really killed. - EXPECT_TRUE(IsProcessDead(child_process)); - base::CloseProcessHandle(child_process); -} - -MULTIPROCESS_TEST_MAIN(process_watcher_test_never_die) { - while (1) { - sleep(500); - } - return 0; -} - -TEST_F(ProcessWatcherTest, ImmediateTermination) { - base::ProcessHandle child_process = - SpawnChild("process_watcher_test_die_immediately", false); - ASSERT_TRUE(child_process); - // Give it time to die. - sleep(2); - ProcessWatcher::EnsureProcessTerminated(child_process); - - // Check that process was really killed. - EXPECT_TRUE(IsProcessDead(child_process)); - base::CloseProcessHandle(child_process); -} - -MULTIPROCESS_TEST_MAIN(process_watcher_test_die_immediately) { - return 0; -} diff --git a/content/common/process_watcher_win.cc b/content/common/process_watcher_win.cc deleted file mode 100644 index e37dd69..0000000 --- a/content/common/process_watcher_win.cc +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "content/common/process_watcher.h" - -#include "base/memory/scoped_ptr.h" -#include "base/message_loop.h" -#include "base/process_util.h" -#include "base/win/object_watcher.h" -#include "content/public/common/result_codes.h" - -// Maximum amount of time (in milliseconds) to wait for the process to exit. -static const int kWaitInterval = 2000; - -namespace { - -class TimerExpiredTask : public Task, - public base::win::ObjectWatcher::Delegate { - public: - explicit TimerExpiredTask(base::ProcessHandle process) : process_(process) { - watcher_.StartWatching(process_, this); - } - - virtual ~TimerExpiredTask() { - if (process_) { - KillProcess(); - DCHECK(!process_) << "Make sure to close the handle."; - } - } - - // Task --------------------------------------------------------------------- - - virtual void Run() { - if (process_) - KillProcess(); - } - - // MessageLoop::Watcher ----------------------------------------------------- - - virtual void OnObjectSignaled(HANDLE object) { - // When we're called from KillProcess, the ObjectWatcher may still be - // watching. the process handle, so make sure it has stopped. - watcher_.StopWatching(); - - CloseHandle(process_); - process_ = NULL; - } - - private: - void KillProcess() { - // OK, time to get frisky. We don't actually care when the process - // terminates. We just care that it eventually terminates, and that's what - // TerminateProcess should do for us. Don't check for the result code since - // it fails quite often. This should be investigated eventually. - base::KillProcess(process_, content::RESULT_CODE_HUNG, false); - - // Now, just cleanup as if the process exited normally. - OnObjectSignaled(process_); - } - - // The process that we are watching. - base::ProcessHandle process_; - - base::win::ObjectWatcher watcher_; - - DISALLOW_COPY_AND_ASSIGN(TimerExpiredTask); -}; - -} // namespace - -// static -void ProcessWatcher::EnsureProcessTerminated(base::ProcessHandle process) { - DCHECK(process != GetCurrentProcess()); - - // If already signaled, then we are done! - if (WaitForSingleObject(process, 0) == WAIT_OBJECT_0) { - CloseHandle(process); - return; - } - - MessageLoop::current()->PostDelayedTask(FROM_HERE, - new TimerExpiredTask(process), - kWaitInterval); -} diff --git a/content/content_common.gypi b/content/content_common.gypi index 9471689..9003e29 100644 --- a/content/content_common.gypi +++ b/content/content_common.gypi @@ -205,10 +205,6 @@ 'common/plugin_carbon_interpose_constants_mac.cc', 'common/plugin_carbon_interpose_constants_mac.h', 'common/plugin_messages.h', - 'common/process_watcher.h', - 'common/process_watcher_mac.cc', - 'common/process_watcher_posix.cc', - 'common/process_watcher_win.cc', 'common/quota_messages.h', 'common/quota_dispatcher.cc', 'common/quota_dispatcher.h', @@ -276,9 +272,6 @@ ], }], ['OS=="mac"', { - 'sources!': [ - 'common/process_watcher_posix.cc', - ], 'link_settings': { 'mac_bundle_resources': [ 'common/common.sb', diff --git a/content/content_tests.gypi b/content/content_tests.gypi index aa97203..26c4249 100644 --- a/content/content_tests.gypi +++ b/content/content_tests.gypi @@ -226,7 +226,6 @@ 'common/hi_res_timer_manager_unittest.cc', 'common/net/url_fetcher_impl_unittest.cc', 'common/page_zoom_unittest.cc', - 'common/process_watcher_unittest.cc', 'common/resource_dispatcher_unittest.cc', 'gpu/gpu_info_collector_unittest.cc', 'gpu/gpu_info_collector_unittest_win.cc', @@ -262,12 +261,6 @@ '../dbus/dbus.gyp:dbus_test_support', ], }], - ['os_posix!=1', { - 'sources!': [ - # TODO(port): port those unit tests. - 'common/process_watcher_unittest.cc', - ], - }], ['OS=="win" and win_use_allocator_shim==1', { 'dependencies': [ '../base/allocator/allocator.gyp:allocator', |