Clean up MojoShellConnection in render processes

MojoShellConnectionImpl ultimately owns a RunnerConnection, which in turn spins up a new thread. If we don't explicitly tear this down then we leak that thread. BUG=590472 [email protected] [email protected] Review URL: https://codereview.chromium.org/1748093002 . Cr-Commit-Position: refs/heads/master@{#378394} (cherry picked from commit 6cb09bb) Review URL: https://codereview.chromium.org/1755843002 . Cr-Commit-Position: refs/branch-heads/2661@{crosswalk-project#34} Cr-Branched-From: ef6f6ae-refs/heads/master@{#378081}
huningxin · Mar 1, 2016 · 064c42a · 064c42a
1 parent ad597d4
commit 064c42a
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 8 deletions.
diff --git a/content/renderer/renderer_main.cc b/content/renderer/renderer_main.cc
@@ -219,6 +219,9 @@ int RendererMain(const MainFunctionParams& parameters) {
       base::MessageLoop::current()->Run();
       TRACE_EVENT_ASYNC_END0("toplevel", "RendererMain.START_MSG_LOOP", 0);
     }
+
+    MojoShellConnectionImpl::Destroy();
+
 #if defined(LEAK_SANITIZER)
     // Run leak detection before RenderProcessImpl goes out of scope. This helps
     // ignore shutdown-only leaks.

diff --git a/mojo/shell/runner/host/child_process_host.cc b/mojo/shell/runner/host/child_process_host.cc
@@ -85,22 +85,28 @@ int ChildProcessHost::Join() {
     start_child_process_event_.Wait();
 
   controller_ = mojom::ChildControllerPtr();
-  DCHECK(child_process_.IsValid());
-
-  int rv = -1;
-  LOG_IF(ERROR, !child_process_.WaitForExit(&rv))
-      << "Failed to wait for child process";
+  // This host may be hosting a child process whose lifetime is controlled
+  // elsewhere. In this case we have no known process handle to wait on.
+  if (child_process_.IsValid()) {
+    int rv = -1;
+    LOG_IF(ERROR, !child_process_.WaitForExit(&rv))
+        << "Failed to wait for child process";
 
-  child_process_.Close();
+    child_process_.Close();
+    return rv;
+  }
 
-  return rv;
+  return 0;
 }
 
 void ChildProcessHost::StartApp(
     InterfaceRequest<mojom::ShellClient> request,
     const mojom::ChildController::StartAppCallback& on_app_complete) {
   DCHECK(controller_);
 
+  // In this case the process must have already been launched.
+  start_child_process_event_.Signal();
+
   on_app_complete_ = on_app_complete;
   controller_->StartApp(
       std::move(request),

diff --git a/mojo/shell/runner/host/out_of_process_native_runner.cc b/mojo/shell/runner/host/out_of_process_native_runner.cc
@@ -67,7 +67,8 @@ void OutOfProcessNativeRunner::AppCompleted(int32_t result) {
   // This object may be deleted by this callback.
   base::Closure app_completed_callback = app_completed_callback_;
   app_completed_callback_.Reset();
-  app_completed_callback.Run();
+  if (!app_completed_callback.is_null())
+    app_completed_callback.Run();
 }
 
 void OutOfProcessNativeRunner::OnProcessLaunched(