Bug 1277064 - Use ConcurrentHashMap for profile cache; r=snorp
authorJim Chen <nchen@mozilla.com>
Fri, 03 Jun 2016 12:01:17 -0400
changeset 375206 e8d8e9ee556559be8abf1944099617970d6a07ff
parent 375205 465fe1502bf49a376d9237bd1438d8a729adb770
child 375207 91a825772c31d0fde133f44d5739e784613835b6
push id20189
push userjlund@mozilla.com
push dateFri, 03 Jun 2016 17:40:55 +0000
reviewerssnorp
bugs1277064
milestone49.0a1
Bug 1277064 - Use ConcurrentHashMap for profile cache; r=snorp Use ConcurrentHashMap for the profile cache so that accessing the cache won't cause inadvertent deadlocks.
mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
@@ -34,19 +34,19 @@ import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.nio.charset.Charset;
 import java.util.Enumeration;
-import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 public final class GeckoProfile {
     private static final String LOGTAG = "GeckoProfile";
 
     // The path in the profile to the file containing the client ID.
     private static final String CLIENT_ID_FILE_PATH = "datareporting/state.json";
@@ -74,17 +74,19 @@ public final class GeckoProfile {
 
     // Session store
     private static final String SESSION_FILE = "sessionstore.js";
     private static final String SESSION_FILE_BACKUP = "sessionstore.bak";
     private static final long MAX_BACKUP_FILE_AGE = 1000 * 3600 * 24; // 24 hours
 
     private boolean mOldSessionDataProcessed = false;
 
-    private static final HashMap<String, GeckoProfile> sProfileCache = new HashMap<String, GeckoProfile>();
+    private static final ConcurrentHashMap<String, GeckoProfile> sProfileCache =
+            new ConcurrentHashMap<String, GeckoProfile>(
+                    /* capacity */ 4, /* load factor */ 0.75f, /* concurrency */ 2);
     private static String sDefaultProfileName;
 
     private final String mName;
     private final File mMozillaDir;
     private final Context mApplicationContext;
 
     private final BrowserDB mDB;
 
@@ -157,17 +159,17 @@ public final class GeckoProfile {
         }
     }
 
     public static GeckoProfile get(Context context) {
         return get(context, null, null, null);
     }
 
     public static GeckoProfile get(Context context, String profileName) {
-        synchronized (sProfileCache) {
+        if (profileName != null) {
             GeckoProfile profile = sProfileCache.get(profileName);
             if (profile != null)
                 return profile;
         }
         return get(context, profileName, (File)null);
     }
 
     @RobocopTarget
@@ -237,61 +239,66 @@ public final class GeckoProfile {
         } else if (profileName == null) {
             // If only profile dir was passed in, use custom (anonymous) profile.
             profileName = CUSTOM_PROFILE;
 
         } else if (AppConstants.DEBUG_BUILD) {
             Log.v(LOGTAG, "Fetching profile: '" + profileName + "', '" + profileDir + "'");
         }
 
+        // We require the profile dir to exist if specified, so create it here if needed.
+        final boolean init = profileDir != null && profileDir.mkdirs();
+
         // Actually try to look up the profile.
-        synchronized (sProfileCache) {
-            // We require the profile dir to exist if specified, so create it here if needed.
-            final boolean init = profileDir != null && profileDir.mkdirs();
+        GeckoProfile profile = sProfileCache.get(profileName);
+        GeckoProfile newProfile = null;
+
+        if (profile == null) {
+            try {
+                newProfile = new GeckoProfile(context, profileName, profileDir, dbFactory);
+            } catch (NoMozillaDirectoryException e) {
+                // We're unable to do anything sane here.
+                throw new RuntimeException(e);
+            }
 
-            GeckoProfile profile = sProfileCache.get(profileName);
-            if (profile == null) {
-                try {
-                    profile = new GeckoProfile(context, profileName, profileDir, dbFactory);
-                } catch (NoMozillaDirectoryException e) {
-                    // We're unable to do anything sane here.
-                    throw new RuntimeException(e);
-                }
-                sProfileCache.put(profileName, profile);
+            profile = sProfileCache.putIfAbsent(profileName, newProfile);
+        }
+
+        if (profile == null) {
+            profile = newProfile;
 
-            } else if (profileDir != null) {
-                // We have an existing profile but was given an alternate directory.
-                boolean consistent = false;
-                try {
-                    consistent = profile.getDir().getCanonicalPath().equals(
-                            profileDir.getCanonicalPath());
-                } catch (final IOException e) {
+        } else if (profileDir != null) {
+            // We have an existing profile but was given an alternate directory.
+            boolean consistent = false;
+            try {
+                consistent = profile.mProfileDir != null &&
+                        profile.mProfileDir.getCanonicalPath().equals(profileDir.getCanonicalPath());
+            } catch (final IOException e) {
+            }
+
+            if (!consistent) {
+                if (!sAcceptDirectoryChanges || !profileDir.isDirectory()) {
+                    throw new IllegalStateException(
+                            "Refusing to reuse profile with a different directory.");
                 }
 
-                if (!consistent) {
-                    if (!sAcceptDirectoryChanges || !profileDir.isDirectory()) {
-                        throw new IllegalStateException(
-                                "Refusing to reuse profile with a different directory.");
-                    }
+                if (AppConstants.RELEASE_BUILD) {
+                    Log.e(LOGTAG, "Release build trying to switch out profile dir. " +
+                                  "This is an error, but let's do what we can.");
+                }
+                profile.setDir(profileDir);
+            }
+        }
 
-                    if (AppConstants.RELEASE_BUILD) {
-                        Log.e(LOGTAG, "Release build trying to switch out profile dir. " +
-                                      "This is an error, but let's do what we can.");
-                    }
-                    profile.setDir(profileDir);
-                }
-            }
+        if (init) {
+            // Initialize the profile directory if we had to create it.
+            profile.enqueueInitialization(profileDir);
+        }
 
-            if (init) {
-                // Initialize the profile directory if we had to create it.
-                profile.enqueueInitialization(profileDir);
-            }
-
-            return profile;
-        }
+        return profile;
     }
 
     // Currently unused outside of testing.
     @RobocopTarget
     public static boolean removeProfile(final Context context, final GeckoProfile profile) {
         final boolean success = profile.remove();
 
         if (success) {