[Cronet] Avoid crashing when CronetEngines are created on multiple threads

components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.net;
 
 import android.content.Context;
 import android.os.Handler;
 import android.os.Looper;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.annotations.JNINamespace;
 
 /**
  * CronetLibraryLoader loads and initializes native library on main thread.
  */
 @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't want it to require locking
just for the sake of an assert.

+    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 to the main thread, and
shouldn't be accessed on other threads (so I like the fact that it has main
thread in its name).  If you feel strongly let me know and I'd be glad to change
it.

 
     /**
      * Ensure that native library is loaded and initialized. Can be called from
      * any thread, the load and initialization is performed on main thread.
      */
     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(
                       "Expected Cronet version number %s, "
                               + "actual version number %s.",
                       Version.CRONET_VERSION,
                       nativeGetCronetVersion()));
             }
             Log.i(TAG, "Cronet version: %s, arch: %s",
                     Version.CRONET_VERSION, System.getProperty("os.arch"));
             ContextUtils.initApplicationContext(context.getApplicationContext());
             // 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.
             if (Looper.getMainLooper() == Looper.myLooper()) {
                 task.run();
             } else {
                 // The initOnMainThread will complete on the main thread prior
                 // 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 doesn't just initialize main thread.

[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
         // embedders do not have API access to create network change
         // observers. Existing observers in the net stack do not
         // perform expensive work.
         NetworkChangeNotifier.registerToReceiveNotificationsAlways();
         // registerToReceiveNotificationsAlways() is called before the native
         // NetworkChangeNotifierAndroid is created, so as to avoid receiving
         // 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.
     private static native void nativeCronetInitOnMainThread();
     private static native String nativeGetCronetVersion();
 }