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

Unified Diff: components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java

Issue 1926683003: [Cronet] Avoid crashing when CronetEngines are created on multiple threads (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 8 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 | components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java
diff --git a/components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java b/components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java
index 0ce0b5430f2fcfe233de5cb98a9ce86e3ef0da91..d0b437aea789156e3ce1fb614b547a7caf8ac6a5 100644
--- a/components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java
+++ b/components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java
@@ -17,12 +17,13 @@ import org.chromium.base.annotations.JNINamespace;
*/
@JNINamespace("cronet")
class CronetLibraryLoader {
- /**
- * Synchronize access to sInitTaskPosted and initialization routine.
- */
+ // Synchronize initialization.
private static final Object sLoadLock = new Object();
private static final String TAG = "CronetLibraryLoader";
- private static boolean sInitTaskPosted = false;
+ // Has library loading commenced? Setting guarded by sLoadLock.
mef 2016/04/27 16:43:47 add @GuardedBy?
pauljensen 2016/04/27 16:56:42 I read it in an assert on another thread and I don
+ private static volatile boolean sInitStarted = false;
+ // Has ensureMainThreadInitialized() completed? Only accessed on main thread.
+ private static boolean sMainThreadInitDone = false;
mef 2016/04/27 16:43:47 Maybe call it sInitDone?
pauljensen 2016/04/27 16:56:42 I'd rather not as this variable is very specific t
/**
* Ensure that native library is loaded and initialized. Can be called from
@@ -31,9 +32,10 @@ class CronetLibraryLoader {
public static void ensureInitialized(
final Context context, final CronetEngine.Builder builder) {
synchronized (sLoadLock) {
- if (sInitTaskPosted) {
+ if (sInitStarted) {
return;
}
+ sInitStarted = true;
builder.loadLibrary();
if (!Version.CRONET_VERSION.equals(nativeGetCronetVersion())) {
throw new RuntimeException(String.format(
@@ -48,7 +50,7 @@ class CronetLibraryLoader {
// Init native Chromium CronetEngine on Main UI thread.
Runnable task = new Runnable() {
public void run() {
- initOnMainThread(context);
+ ensureMainThreadInitialized(context);
}
};
// Run task immediately or post it to the UI thread.
@@ -59,11 +61,20 @@ class CronetLibraryLoader {
// to other tasks posted to the main thread.
new Handler(Looper.getMainLooper()).post(task);
}
- sInitTaskPosted = true;
}
}
- private static void initOnMainThread(final Context context) {
+ /**
+ * Ensure that the main thread initialization has completed. Can only be called from
+ * the main thread. Ensures that the NetworkChangeNotifier is initialzied and the
+ * main thread native MessageLoop is initialized.
+ */
+ public static void ensureMainThreadInitialized(Context context) {
kapishnikov 2016/04/27 15:53:17 Don't need to be public.
mef 2016/04/27 16:43:47 Maybe call it 'ensureInitializedOnMainThread'? It
pauljensen 2016/04/27 16:56:42 Done, also done for ensureLibraryLoaded.
pauljensen 2016/04/27 16:56:42 Done.
+ assert sInitStarted;
+ assert Looper.getMainLooper() == Looper.myLooper();
+ if (sMainThreadInitDone) {
+ return;
+ }
NetworkChangeNotifier.init(context);
// Registers to always receive network notifications. Note
// that this call is fine for Cronet because Cronet
@@ -76,6 +87,7 @@ class CronetLibraryLoader {
// the undesired initial network change observer notification, which
// will cause active requests to fail with ERR_NETWORK_CHANGED.
nativeCronetInitOnMainThread();
+ sMainThreadInitDone = true;
}
// Native methods are implemented in cronet_library_loader.cc.
« no previous file with comments | « no previous file | components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698