Bug 1542581 - Block child process termination until the handler is fully initialized. r=froydnj, a=lizzard
authorGabriele Svelto <gsvelto@mozilla.com>
Wed, 17 Apr 2019 18:55:14 +0000
changeset 523272 5e9f1cf0693663467059f729d897e2f23b4631e9
parent 523271 7ccaf23243177f7a103a6155f48f006ad0db545e
child 523273 50707a995cd56232e52b0e3453ec73c17e1339e6
push id11123
push userryanvm@gmail.com
push dateMon, 22 Apr 2019 13:21:32 +0000
treeherdermozilla-beta@5e9f1cf06936 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj, lizzard
bugs1542581
milestone67.0
Bug 1542581 - Block child process termination until the handler is fully initialized. r=froydnj, a=lizzard Differential Revision: https://phabricator.services.mozilla.com/D27231
toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.cc
--- a/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.cc
+++ b/toolkit/crashreporter/breakpad-client/windows/crash_generation/crash_generation_server.cc
@@ -778,32 +778,33 @@ bool CrashGenerationServer::AddClient(Cl
                                    client_info,
                                    INFINITE,
                                    kDumpRequestThreadFlags)) {
     return false;
   }
 
   client_info->set_dump_request_wait_handle(request_wait_handle);
 
-  // OnClientEnd will be called when the client process terminates.
-  HANDLE process_wait_handle = NULL;
-  if (!RegisterWaitForSingleObject(&process_wait_handle,
-                                   client_info->process_handle(),
-                                   OnClientEnd,
-                                   client_info,
-                                   INFINITE,
-                                   WT_EXECUTEONLYONCE)) {
-    return false;
-  }
-
-  client_info->set_process_exit_wait_handle(process_wait_handle);
-
   // New scope to hold the lock for the shortest time.
   {
     AutoCriticalSection lock(&sync_);
+
+    // OnClientEnd will be called when the client process terminates.
+    HANDLE process_wait_handle = NULL;
+    if (!RegisterWaitForSingleObject(&process_wait_handle,
+                                     client_info->process_handle(),
+                                     OnClientEnd,
+                                     client_info,
+                                     INFINITE,
+                                     WT_EXECUTEONLYONCE)) {
+      return false;
+    }
+
+    client_info->set_process_exit_wait_handle(process_wait_handle);
+
     if (shutting_down_) {
       // If server is shutting down, don't add new clients
       return false;
     }
     clients_.push_back(client_info);
   }
 
   return true;
@@ -862,16 +863,18 @@ void CrashGenerationServer::HandleClient
     if (shutting_down_) {
       // The crash generation server is shutting down and as part of the
       // shutdown process it will delete all clients from the clients_ list.
       return;
     }
     clients_.remove(client_info);
   }
 
+  AutoCriticalSection lock(&sync_);
+
   // Explicitly unregister the process exit wait using the non-blocking method.
   // Otherwise, the destructor will attempt to unregister it using the blocking
   // method which will lead to a deadlock because it is being called from the
   // callback of the same wait operation
   client_info->UnregisterProcessExitWait(false);
 
   delete client_info;
 }