diff options
| -rw-r--r-- | base/process_util.h | 10 | ||||
| -rw-r--r-- | base/process_util_posix.cc | 17 | ||||
| -rw-r--r-- | base/process_util_win.cc | 6 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/browser_render_process_host.cc | 15 | ||||
| -rw-r--r-- | chrome/chrome.gyp | 4 | ||||
| -rw-r--r-- | chrome/common/child_process_host.cc | 2 | ||||
| -rw-r--r-- | chrome/common/common.vcproj | 2 | ||||
| -rw-r--r-- | chrome/common/process_watcher.cc (renamed from chrome/common/process_watcher_win.cc) | 0 | ||||
| -rw-r--r-- | chrome/common/process_watcher_posix.cc | 79 | ||||
| -rw-r--r-- | chrome/common/temp_scaffolding_stubs.cc | 5 |
10 files changed, 18 insertions, 122 deletions
diff --git a/base/process_util.h b/base/process_util.h index a692870..9eb8690 100644 --- a/base/process_util.h +++ b/base/process_util.h @@ -172,13 +172,9 @@ bool KillProcessById(ProcessId process_id, int exit_code, bool wait); #endif // Get the termination status (exit code) of the process and return true if the -// status indicates the process crashed. |child_exited| is set to true iff the -// child process has terminated. (|child_exited| may be NULL.) -// -// On Windows, it is an error to call this if the process hasn't terminated -// yet. On POSIX, |child_exited| is set correctly since we detect terminate in -// a different manner on POSIX. -bool DidProcessCrash(bool* child_exited, ProcessHandle handle); +// status indicates the process crashed. It is an error to call this if the +// process hasn't terminated yet. +bool DidProcessCrash(ProcessHandle handle); // Waits for process to exit. In POSIX systems, if the process hasn't been // signaled then puts the exit code in |exit_code|; otherwise it's considered diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc index 87abbb8..562b8e9 100644 --- a/base/process_util_posix.cc +++ b/base/process_util_posix.cc @@ -140,24 +140,13 @@ void RaiseProcessToHighPriority() { // setpriority() or sched_getscheduler, but these all require extra rights. } -bool DidProcessCrash(bool* child_exited, ProcessHandle handle) { +bool DidProcessCrash(ProcessHandle handle) { int status; - const int result = waitpid(handle, &status, WNOHANG); - if (result == -1) { - LOG(ERROR) << "waitpid failed with errno:" << errno; - if (child_exited) - *child_exited = false; - return false; - } else if (result == 0) { - // the child hasn't exited yet. - if (child_exited) - *child_exited = false; + if (waitpid(handle, &status, WNOHANG)) { + // I feel like dancing! return false; } - if (child_exited) - *child_exited = true; - if (WIFSIGNALED(status)) { switch(WTERMSIG(status)) { case SIGSEGV: diff --git a/base/process_util_win.cc b/base/process_util_win.cc index fc05664..13fec9a 100644 --- a/base/process_util_win.cc +++ b/base/process_util_win.cc @@ -270,12 +270,8 @@ bool KillProcess(ProcessHandle process, int exit_code, bool wait) { return result; } -bool DidProcessCrash(bool* child_exited, ProcessHandle handle) { +bool DidProcessCrash(ProcessHandle handle) { DWORD exitcode = 0; - - if (child_exited) - *child_exited = true; // On Windows it an error to call this function if - // the child hasn't already exited. if (!::GetExitCodeProcess(handle, &exitcode)) { NOTREACHED(); return false; diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc index 462b382..a75d9de 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.cc +++ b/chrome/browser/renderer_host/browser_render_process_host.cc @@ -661,24 +661,13 @@ void BrowserRenderProcessHost::OnChannelError() { DCHECK(process_.handle()); DCHECK(channel_.get()); - bool child_exited; - if (base::DidProcessCrash(&child_exited, process_.handle())) { + if (base::DidProcessCrash(process_.handle())) { NotificationService::current()->Notify( NotificationType::RENDERER_PROCESS_CRASHED, Source<RenderProcessHost>(this), NotificationService::NoDetails()); } - // If the process crashed, then the kernel closed the socket for it and so - // the child has already died by the time we get here. Since DidProcessCrash - // called waitpid with WNOHANG, it'll reap the process. However, if - // DidProcessCrash didn't reap the child, we'll need to in - // ~BrowserRenderProcessHost via ProcessWatcher. So we can't close the handle - // here. - // - // This is moot on Windows where |child_exited| will always be true. - if (child_exited) - process_.Close(); - + process_.Close(); channel_.reset(); // This process should detach all the listeners, causing the object to be diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp index fa9d7fd..8ccab4d 100644 --- a/chrome/chrome.gyp +++ b/chrome/chrome.gyp @@ -250,8 +250,7 @@ 'common/pref_names.h', 'common/pref_service.cc', 'common/pref_service.h', - 'common/process_watcher_posix.cc', - 'common/process_watcher_win.cc', + 'common/process_watcher.cc', 'common/process_watcher.h', 'common/property_bag.cc', 'common/property_bag.h', @@ -347,6 +346,7 @@ 'common/classfactory.cc', 'common/drag_drop_types.cc', 'common/os_exchange_data.cc', + 'common/process_watcher.cc', ], }], ], diff --git a/chrome/common/child_process_host.cc b/chrome/common/child_process_host.cc index e7a987f..7f934b0 100644 --- a/chrome/common/child_process_host.cc +++ b/chrome/common/child_process_host.cc @@ -117,7 +117,7 @@ void ChildProcessHost::OnWaitableEventSignaled(base::WaitableEvent *event) { DCHECK(handle()); DCHECK_EQ(object, handle()); - bool did_crash = base::DidProcessCrash(NULL, object); + bool did_crash = base::DidProcessCrash(object); if (did_crash) { // Report that this child process crashed. Notify(NotificationType::CHILD_PROCESS_CRASHED); diff --git a/chrome/common/common.vcproj b/chrome/common/common.vcproj index ae39225..5a26a5e 100644 --- a/chrome/common/common.vcproj +++ b/chrome/common/common.vcproj @@ -694,7 +694,7 @@ > </File> <File - RelativePath=".\process_watcher_win.cc" + RelativePath=".\process_watcher.cc" > </File> <File diff --git a/chrome/common/process_watcher_win.cc b/chrome/common/process_watcher.cc index 1ee7edf..1ee7edf 100644 --- a/chrome/common/process_watcher_win.cc +++ b/chrome/common/process_watcher.cc diff --git a/chrome/common/process_watcher_posix.cc b/chrome/common/process_watcher_posix.cc deleted file mode 100644 index ef73521..0000000 --- a/chrome/common/process_watcher_posix.cc +++ /dev/null @@ -1,79 +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 "chrome/common/process_watcher.h" - -#include <errno.h> -#include <signal.h> -#include <sys/types.h> -#include <sys/wait.h> - -#include "base/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 int result = waitpid(child, NULL, WNOHANG); - if (result == -1) { - 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: - explicit BackgroundReaper(pid_t child) - : child_(child) { - } - - void ThreadMain() { - WaitForChildToDie(); - delete this; - } - - void WaitForChildToDie() { - // 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. - - // Waits 0.5 * 4 = 2 seconds. - for (unsigned i = 0; i < 4; ++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. - int result; - do { - result = waitpid(child_, NULL, 0); - } while (result == -1 && errno == EINTR); - } else { - LOG(ERROR) << "While waiting for " << child_ << " to terminate we" - << " failed to deliver a SIGKILL signal (" << errno << ")."; - } - } - - private: - const pid_t child_; - - 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; - - BackgroundReaper* reaper = new BackgroundReaper(process); - PlatformThread::CreateNonJoinable(0, reaper); -} diff --git a/chrome/common/temp_scaffolding_stubs.cc b/chrome/common/temp_scaffolding_stubs.cc index 1245200..9aeb0cc 100644 --- a/chrome/common/temp_scaffolding_stubs.cc +++ b/chrome/common/temp_scaffolding_stubs.cc @@ -232,6 +232,11 @@ LoginHandler* CreateLoginPrompt(net::AuthChallengeInfo* auth_info, return NULL; } +void ProcessWatcher::EnsureProcessTerminated(int) { + NOTIMPLEMENTED(); +} + + //-------------------------------------------------------------------------- namespace webkit_glue { |