diff options
-rw-r--r-- | base/process_util.h | 4 | ||||
-rw-r--r-- | base/process_util_unittest.cc | 18 | ||||
-rw-r--r-- | base/process_util_win.cc | 22 | ||||
-rw-r--r-- | chrome/browser/child_process_launcher.cc | 2 | ||||
-rw-r--r-- | chrome/browser/nacl_host/nacl_process_host.cc | 6 |
5 files changed, 38 insertions, 14 deletions
diff --git a/base/process_util.h b/base/process_util.h index 705b90b..20f16a7 100644 --- a/base/process_util.h +++ b/base/process_util.h @@ -264,10 +264,6 @@ bool KillProcessById(ProcessId process_id, int exit_code, bool wait); // 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); // Waits for process to exit. In POSIX systems, if the process hasn't been diff --git a/base/process_util_unittest.cc b/base/process_util_unittest.cc index 296aaa9..b4d9b9f 100644 --- a/base/process_util_unittest.cc +++ b/base/process_util_unittest.cc @@ -78,6 +78,24 @@ TEST_F(ProcessUtilTest, KillSlowChild) { base::CloseProcessHandle(handle); } +TEST_F(ProcessUtilTest, DidProcessCrash) { + remove("SlowChildProcess.die"); + ProcessHandle handle = this->SpawnChild(L"SlowChildProcess"); + ASSERT_NE(base::kNullProcessHandle, handle); + + bool child_exited = true; + EXPECT_FALSE(base::DidProcessCrash(&child_exited, handle)); + EXPECT_FALSE(child_exited); + + FILE *fp = fopen("SlowChildProcess.die", "w"); + fclose(fp); + EXPECT_TRUE(base::WaitForSingleProcess(handle, 5000)); + + EXPECT_FALSE(base::DidProcessCrash(&child_exited, handle)); + + base::CloseProcessHandle(handle); +} + // Ensure that the priority of a process is restored correctly after // backgrounding and restoring. // Note: a platform may not be willing or able to lower the priority of diff --git a/base/process_util_win.cc b/base/process_util_win.cc index 05d3d60..a845ac7 100644 --- a/base/process_util_win.cc +++ b/base/process_util_win.cc @@ -318,19 +318,33 @@ bool KillProcess(ProcessHandle process, int exit_code, bool wait) { bool DidProcessCrash(bool* child_exited, 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(); + // Assume the child has exited. + if (child_exited) + *child_exited = true; return false; } if (exitcode == STILL_ACTIVE) { - // The process is likely not dead or it used 0x103 as exit code. + DWORD wait_result = WaitForSingleObject(handle, 0); + if (wait_result == WAIT_TIMEOUT) { + if (child_exited) + *child_exited = false; + return false; + } + + DCHECK_EQ(WAIT_OBJECT_0, wait_result); + + // Strange, the process used 0x103 (STILL_ACTIVE) as exit code. NOTREACHED(); + return false; } + // We're sure the child has exited. + if (child_exited) + *child_exited = true; + // Warning, this is not generic code; it heavily depends on the way // the rest of the code kills a process. diff --git a/chrome/browser/child_process_launcher.cc b/chrome/browser/child_process_launcher.cc index ff555fe..206d8a7 100644 --- a/chrome/browser/child_process_launcher.cc +++ b/chrome/browser/child_process_launcher.cc @@ -312,8 +312,6 @@ bool ChildProcessLauncher::DidProcessCrash() { // DidProcessCrash called waitpid with WNOHANG, it'll reap the process. // However, if DidProcessCrash didn't reap the child, we'll need to in // Terminate 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) context_->process_.Close(); diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc index 1e3bcff..2648b3e 100644 --- a/chrome/browser/nacl_host/nacl_process_host.cc +++ b/chrome/browser/nacl_host/nacl_process_host.cc @@ -118,10 +118,8 @@ void NaClProcessHost::OnProcessLaunchedByBroker(base::ProcessHandle handle) { } bool NaClProcessHost::DidChildCrash() { - if (running_on_wow64_) { - bool child_exited; - return base::DidProcessCrash(&child_exited, handle()); - } + if (running_on_wow64_) + return base::DidProcessCrash(NULL, handle()); return ChildProcessHost::DidChildCrash(); } |