Bug 920397 part 1 - base::CloseProcessHandle() checks result in debug builds and is used everywhere a process handle is closed. r=bsmedberg
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 01 Oct 2013 10:56:15 +1000
changeset 149396 571e97a6ae5442bad4cd2fffd5d0eebaad33e01a
parent 149395 2466893f18a72c83e618c9a30c3e8f13f5bfd707
child 149397 10c891d5b4e3b50c4dc5bec0f52f7d77b21ad615
push id25386
push useremorley@mozilla.com
push dateTue, 01 Oct 2013 09:29:22 +0000
treeherdermozilla-central@6856c45f3688 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs920397
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 920397 part 1 - base::CloseProcessHandle() checks result in debug builds and is used everywhere a process handle is closed. r=bsmedberg
ipc/chromium/src/base/process_util_win.cc
ipc/chromium/src/base/process_win.cc
ipc/chromium/src/chrome/common/process_watcher_win.cc
--- a/ipc/chromium/src/base/process_util_win.cc
+++ b/ipc/chromium/src/base/process_util_win.cc
@@ -94,17 +94,21 @@ bool OpenPrivilegedProcessHandle(Process
   if (result == INVALID_HANDLE_VALUE)
     return false;
 
   *handle = result;
   return true;
 }
 
 void CloseProcessHandle(ProcessHandle process) {
-  CloseHandle(process);
+  // closing a handle twice on Windows can be catastrophic - after the first
+  // close the handle value may be reused, so the second close will kill that
+  // other new handle.
+  BOOL ok = CloseHandle(process);
+  DCHECK(ok);
 }
 
 // Helper for GetProcId()
 bool GetProcIdViaGetProcessId(ProcessHandle process, DWORD* id) {
   // Dynamically get a pointer to GetProcessId().
   typedef DWORD (WINAPI *GetProcessIdFunction)(HANDLE);
   static GetProcessIdFunction GetProcessIdPtr = NULL;
   static bool initialize_get_process_id = true;
--- a/ipc/chromium/src/base/process_win.cc
+++ b/ipc/chromium/src/base/process_win.cc
@@ -7,17 +7,17 @@
 #include "base/process_util.h"
 #include "base/scoped_ptr.h"
 
 namespace base {
 
 void Process::Close() {
   if (!process_)
     return;
-  ::CloseHandle(process_);
+  CloseProcessHandle(process_);
   process_ = NULL;
 }
 
 void Process::Terminate(int result_code) {
   if (!process_)
     return;
   ::TerminateProcess(process_, result_code);
 }
--- a/ipc/chromium/src/chrome/common/process_watcher_win.cc
+++ b/ipc/chromium/src/chrome/common/process_watcher_win.cc
@@ -37,17 +37,17 @@ class TimerExpiredTask : public Task, pu
 
   // 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_);
+    base::CloseProcessHandle(process_);
     process_ = NULL;
   }
 
  private:
   void KillProcess() {
     if (base::SysInfo::HasEnvVar(env_vars::kHeadless)) {
      // If running the distributed tests, give the renderer a little time
      // to figure out that the channel is shutdown and unwind.
@@ -80,22 +80,22 @@ class TimerExpiredTask : public Task, pu
 // static
 void ProcessWatcher::EnsureProcessTerminated(base::ProcessHandle process
 					     , bool force
 ) {
   DCHECK(process != GetCurrentProcess());
 
   if (!force) {
     WaitForSingleObject(process, INFINITE);
-    CloseHandle(process);
+    base::CloseProcessHandle(process);
     return;
   }
 
   // If already signaled, then we are done!
   if (WaitForSingleObject(process, 0) == WAIT_OBJECT_0) {
-    CloseHandle(process);
+    base::CloseProcessHandle(process);
     return;
   }
 
   MessageLoop::current()->PostDelayedTask(FROM_HERE,
                                           new TimerExpiredTask(process),
                                           kWaitInterval);
 }