Removed unnecessary potential locking

base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java

 // Copyright 2014 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.base.library_loader;
 
 import android.content.Context;
 import android.os.SystemClock;
 import android.util.Log;
 
@@ skipping 24 matching lines @@
 public class LibraryLoader {
     private static final String TAG = "LibraryLoader";
 
     // Set to true to enable debug logs.
     private static final boolean DEBUG = false;
 
     // Guards all access to the libraries
     private static final Object sLock = new Object();
 
     // The singleton instance of LibraryLoader.
-    private static LibraryLoader sInstance;
+    private static volatile LibraryLoader sInstance;
 
     // One-way switch becomes true when the libraries are loaded.
     private boolean mLoaded;
 
     // One-way switch becomes true when the Java command line is switched to
     // native.
     private boolean mCommandLineSwitched;
 
     // One-way switch becomes true when the libraries are initialized (
     // by calling nativeLibraryLoaded, which forwards to LibraryLoaded(...) in
     // library_loader_hooks.cc).
-    private boolean mInitialized;
+    private volatile boolean mInitialized;
 
     // One-way switches recording attempts to use Relro sharing in the browser.
     // The flags are used to report UMA stats later.
     private boolean mIsUsingBrowserSharedRelros;
     private boolean mLoadAtFixedAddressFailed;
 
     // One-way switch becomes true if the device supports memory mapping the
     // APK file with executable permissions.
     private boolean mMapApkWithExecPermission;
 
     // One-way switch to indicate whether we probe for memory mapping the APK
     // file with executable permissions. We suppress the probe under some
     // conditions.
     // For more, see:
     //   https://code.google.com/p/chromium/issues/detail?id=448084
     private boolean mProbeMapApkWithExecPermission = true;
 
     // One-way switch becomes true if the Chromium library was loaded from the
     // APK file directly.
     private boolean mLibraryWasLoadedFromApk;
 
     // One-way switch becomes false if the Chromium library should be loaded
     // directly from the APK file but it was compressed or not aligned.
     private boolean mLibraryIsMappableInApk = true;
 
     // The type of process the shared library is loaded in.
-    private int mLibraryProcessType;
+    private volatile int mLibraryProcessType;

[michaelbai, 2015/02/25 04:57:12]

You probably should use final instead of volatile because it isn't expected to
be changed after object created.

[ripp, 2015/02/25 08:32:23]

Done.

 
     /**
      * @param libraryProcessType the process the shared library is loaded in. refer to
      *                           LibraryProcessType for possible values.
      * @return LibraryLoader if existing, otherwise create a new one.
      */
     public static LibraryLoader get(int libraryProcessType) throws ProcessInitException {
         synchronized (sLock) {
             if (sInstance != null) {
                 if (sInstance.mLibraryProcessType == libraryProcessType) return sInstance;
@@ skipping 39 matching lines @@
             }
             loadAlreadyLocked(context, shouldDeleteFallbackLibraries);
             initializeAlreadyLocked();
         }
     }
 
     /**
      * Checks if library is fully loaded and initialized.
      */
     public static boolean isInitialized() {
-        synchronized (sLock) {
-            return sInstance != null && sInstance.mInitialized;
-        }
+        return sInstance != null && sInstance.mInitialized;
     }
 
     /**
      * The same as loadNow(null, false), should only be called by
      * non-browser process.
      *
      * @throws ProcessInitException
      */
     public void loadNow() throws ProcessInitException {
         loadNow(null, false);
@@ skipping 213 matching lines @@
 
         // Setup the native command line if necessary.
         if (!mCommandLineSwitched) {
             nativeInitCommandLine(CommandLine.getJavaSwitchesOrNull());
         }
 
         if (!nativeLibraryLoaded()) {
             Log.e(TAG, "error calling nativeLibraryLoaded");
             throw new ProcessInitException(LoaderErrors.LOADER_ERROR_FAILED_TO_REGISTER_JNI);
         }
-        // From this point on, native code is ready to use and checkIsReady()
-        // shouldn't complain from now on (and in fact, it's used by the
-        // following calls).
-        mInitialized = true;
 
         // The Chrome JNI is registered by now so we can switch the Java
         // command line over to delegating to native if it's necessary.
         if (!mCommandLineSwitched) {
             CommandLine.enableNativeProxy();
             mCommandLineSwitched = true;
         }
 
         // From now on, keep tracing in sync with native.
         TraceEvent.registerNativeEnabledObserver();
+
+        // From this point on, native code is ready to use and checkIsReady()
+        // shouldn't complain from now on (and in fact, it's used by the
+        // following calls).
+        // Note that this flag can be accessed asynchronously, so any initialization
+        // must be performed before.
+        mInitialized = true;
     }
 
     // Called after all native initializations are complete.
     public void onNativeInitializationComplete(Context context) {
         recordBrowserProcessHistogram(context);
     }
 
     // Record Chromium linker histogram state for the main browser process. Called from
     // onNativeInitializationComplete().
     private void recordBrowserProcessHistogram(Context context) {
@@ skipping 44 matching lines @@
                                                                  loadAtFixedAddressFailed);
         }
     }
 
     /**
      * @return the process the shared library is loaded in, see the LibraryProcessType
      *         for possible values.
      */
     @CalledByNative
     public static int getLibraryProcessType() {
-        synchronized (sLock) {
-            if (sInstance == null) return LibraryProcessType.PROCESS_UNINITIALIZED;
-            return sInstance.mLibraryProcessType;
-        }
+        if (sInstance == null) return LibraryProcessType.PROCESS_UNINITIALIZED;
+        return sInstance.mLibraryProcessType;
     }
 
     private native void nativeInitCommandLine(String[] initCommandLine);
 
     // Only methods needed before or during normal JNI registration are during System.OnLoad.
     // nativeLibraryLoaded is then called to register everything else.  This process is called
     // "initialization".  This method will be mapped (by generated code) to the LibraryLoaded
     // definition in base/android/library_loader/library_loader_hooks.cc.
     //
     // Return true on success and false on failure.
@@ skipping 12 matching lines @@
     // operation for a renderer process. Indicates whether the linker attempted relro sharing,
     // and if it did, whether the library failed to load at a fixed address.
     private native void nativeRegisterChromiumAndroidLinkerRendererHistogram(
             boolean requestedSharedRelro,
             boolean loadAtFixedAddressFailed);
 
     // Get the version of the native library. This is needed so that we can check we
     // have the right version before initializing the (rest of the) JNI.
     private native String nativeGetVersionNumber();
 }