Fix deadlock in ChildProcessService.onDestroy()

onCreate() spawns the CrMain thread. onDestroy() can happen quickly
after onCreate() (for example if the browser crashes),
so we have to be careful about blocking on any semaphores
wrt the CrMain thread.

The existing cmdline check did not cover all of that.

In addition we have to avoid deadlocks between
nativeShutdownMainThread()and us actually not having ever
entered native main().
(ChildThreadImpl::Shutdown() on Android waits on a
condition variable.)

BUG=448549,457406,457914
NOTRY=True

Review URL: https://codereview.chromium.org/1052133002

Cr-Commit-Position: refs/heads/master@{#323567}

Review URL: https://codereview.chromium.org/1061133004

Cr-Commit-Position: refs/branch-heads/2311@{#441}
Cr-Branched-From: 09b7de5dd7254947cd4306de907274fa63373d48-refs/heads/master@{#317474}

1 files changed, 14 insertions, 4 deletions

diff --git a/content/public/android/java/src/org/chromium/content/app/ChildProcessService.java b/content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
index 5c91035..fae9a41 100644
--- a/content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
+++ b/content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
@@ -31,6 +31,7 @@ import org.chromium.content.common.IChildProcessCallback;
 import org.chromium.content.common.IChildProcessService;
 
 import java.util.ArrayList;
+import java.util.concurrent.Semaphore;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -66,6 +67,8 @@ public class ChildProcessService extends Service {
     // Becomes true once the service is bound. Access must synchronize around mMainThread.
     private boolean mIsBound = false;
 
+    private final Semaphore mActivitySemaphore = new Semaphore(1);
+
     // Binder object used by clients for this service.
     private final IChildProcessService.Stub mBinder = new IChildProcessService.Stub() {
         // NOTE: Implement any IChildProcessService methods here.
@@ -213,8 +216,10 @@ public class ChildProcessService extends Service {
                     nativeInitChildProcess(sContext.get().getApplicationContext(),
                             ChildProcessService.this, fileIds, fileFds,
                             mCpuCount, mCpuFeatures);
-                    ContentMain.start();
-                    nativeExitChildProcess();
+                    if (mActivitySemaphore.tryAcquire()) {
+                        ContentMain.start();
+                        nativeExitChildProcess();
+                    }
                 } catch (InterruptedException e) {
                     Log.w(TAG, MAIN_THREAD_NAME + " startup failed: " + e);
                 } catch (ProcessInitException e) {
@@ -226,11 +231,16 @@ public class ChildProcessService extends Service {
     }
 
     @Override
+    @SuppressFBWarnings("DM_EXIT")
     public void onDestroy() {
         Log.i(TAG, "Destroying ChildProcessService pid=" + Process.myPid());
         super.onDestroy();
-        if (mCommandLineParams == null) {
-            // This process was destroyed before it even started. Nothing more to do.
+        if (mActivitySemaphore.tryAcquire()) {
+            // TODO(crbug.com/457406): This is a bit hacky, but there is no known better solution
+            // as this service will get reused (at least if not sandboxed).
+            // In fact, we might really want to always exit() from onDestroy(), not just from
+            // the early return here.
+            System.exit(0);
             return;
         }
         synchronized (mMainThread) {