Bug 1556083 - Resolve deadlock by using just one lock, not two; r=VladBaicu, a=jcristau
authorPetru Lingurar <petru.lingurar@softvision.ro>
Mon, 24 Jun 2019 17:52:40 +0000
changeset 537103 186a6085f0bc20970bf9ded561a2844b1d176ab1
parent 537102 2638b73811db33a1360300fa5b501fc8b5c84297
child 537104 1c3f96c23a841c7a6e368d73b9ee5b9573d943a3
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersVladBaicu, jcristau
bugs1556083
milestone68.0
Bug 1556083 - Resolve deadlock by using just one lock, not two; r=VladBaicu, a=jcristau Classic deadlock situation possible because getDatabaseHelperForProfile(..) would lock on [PerProfileDatabase] and then try to on [GeckoProfile] while at the same time it would be possible for another thread which already had the [GeckoProfile] lock to call this method and so try to acquire the [PerProfileDatabase] lock. The simplest solution to resolve this and the one I went with is to ensure that one of those threads will not need both locks and it turns out that the getDatabaseHelperForProfile method can easily be refactored to use only the GeckoProfile lock, change which would not significantly increase the block of code synchronized with the same key. Differential Revision: https://phabricator.services.mozilla.com/D35646
mobile/android/base/java/org/mozilla/gecko/db/PerProfileDatabases.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/PerProfileDatabases.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/PerProfileDatabases.java
@@ -56,36 +56,36 @@ public class PerProfileDatabases<T exten
 
         return new File(profileDir, mDatabaseName).getAbsolutePath();
     }
 
     public T getDatabaseHelperForProfile(String profile) {
         return getDatabaseHelperForProfile(profile, false);
     }
 
-    public T getDatabaseHelperForProfile(String profile, boolean isTest) {
-        // Always fall back to default profile if none has been provided.
-        if (profile == null) {
-            profile = GeckoProfile.get(mContext).getName();
-        }
-
-        synchronized (this) {
-            if (mStorages.containsKey(profile)) {
-                return mStorages.get(profile);
+    public T getDatabaseHelperForProfile(String profileName, boolean isTest) {
+        synchronized (GeckoProfile.get(mContext)) {
+            // Always fall back to default profile if none has been provided.
+            if (profileName == null) {
+                profileName = GeckoProfile.get(mContext).getName();
             }
 
-            final String databasePath = isTest ? mDatabaseName : getDatabasePathForProfile(profile);
+            if (mStorages.containsKey(profileName)) {
+                return mStorages.get(profileName);
+            }
+
+            final String databasePath = isTest ? mDatabaseName : getDatabasePathForProfile(profileName);
             if (databasePath == null) {
-                throw new IllegalStateException("Database path is null for profile: " + profile);
+                throw new IllegalStateException("Database path is null for profile: " + profileName);
             }
 
             final T helper = mHelperFactory.makeDatabaseHelper(mContext, databasePath);
             DBUtils.ensureDatabaseIsNotLocked(helper, databasePath);
 
-            mStorages.put(profile, helper);
+            mStorages.put(profileName, helper);
             return helper;
         }
     }
 
     public synchronized void shrinkMemory() {
         for (T t : mStorages.values()) {
             final SQLiteDatabase db = t.getWritableDatabase();
             db.execSQL("PRAGMA shrink_memory");