Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1838)

Unified Diff: content/public/android/java/src/org/chromium/content/app/ChildProcessService.java

Issue 1052133002: Fix deadlock in ChildProcessService.onDestroy() (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: suppress findbugs Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/public/android/java/src/org/chromium/content/app/ChildProcessService.java
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 5c910352a5b0f4637537e888c395de8decc64c59..fae9a41007c0aabff9180ea2e6a2f283d219ed3a 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) {
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698