Removed locks in trf-persona-store that are not needed.

Minor style fixes based on review of previous changes.

6 files changed, 72 insertions, 67 deletions

diff --git a/backends/libsocialweb/lib/swf-persona-store.vala b/backends/libsocialweb/lib/swf-persona-store.vala
index 9ee75352..18e25f13 100644
--- a/backends/libsocialweb/lib/swf-persona-store.vala
+++ b/backends/libsocialweb/lib/swf-persona-store.vala
@@ -318,13 +318,18 @@ public class Swf.PersonaStore : Folks.PersonaStore
     {
       Internal.profiling_start ("preparing Swf.PersonaStore (ID: %s)", this.id);
 
-      if (!this._is_prepared && !this._prepare_pending)
+      if (this._is_prepared || this._prepare_pending)
+        {
+          return;
+        }
+      
+      try
         {
           this._prepare_pending = true;
-
+          
           /* Get the service's capabilities. */
           string[]? caps = null;
-
+          
           try
             {
               caps = yield this._get_static_capabilities ();
@@ -392,7 +397,6 @@ public class Swf.PersonaStore : Folks.PersonaStore
 
           this._contact_view = contact_view;
           this._is_prepared = true;
-          this._prepare_pending = false;
           this.notify_property ("is-prepared");
 
           /* FIXME: for lsw Stores with 0 contacts or badly configured (or
@@ -411,6 +415,10 @@ public class Swf.PersonaStore : Folks.PersonaStore
 
           this._contact_view.start ();
         }
+      finally
+        {
+          this._prepare_pending = false;
+        }
 
       Internal.profiling_end ("preparing Swf.PersonaStore (ID: %s)", this.id);
     }
diff --git a/backends/libsocialweb/sw-backend.vala b/backends/libsocialweb/sw-backend.vala
index db7d7def..6aa80b96 100644
--- a/backends/libsocialweb/sw-backend.vala
+++ b/backends/libsocialweb/sw-backend.vala
@@ -102,18 +102,18 @@ public class Folks.Backends.Sw.Backend : Folks.Backend
           return;
         }
         
-      try
-        {
-          this._prepare_pending = true;
+      /* Hold a ref. on the Backend while we wait for the callback from
+       * this._client.get_services() to prevent the Backend being
+       * destroyed in the mean time. See: bgo#665039. */
+      this.ref ();
 
-          /* Hold a ref. on the Backend while we wait for the callback from
-           * this._client.get_services() to prevent the Backend being
-           * destroyed in the mean time. See: bgo#665039. */
-          this.ref ();
-
-          this._client = new Client ();
-          this._client.get_services((client, services) =>
+      this._client = new Client ();
+      this._client.get_services((client, services) =>
+        {
+          try
             {
+              this._prepare_pending = true;
+        
               foreach (var service_name in services)
                 this.add_service (service_name);
 
@@ -124,12 +124,12 @@ public class Folks.Backends.Sw.Backend : Folks.Backend
               this.notify_property ("is-quiescent");
 
               this.unref ();
-            });
-        }
-      finally
-        {
-          this._prepare_pending = false;
-        }
+            }
+          finally
+            {
+              this._prepare_pending = false;
+            }
+        });
 
       Internal.profiling_end ("preparing Sw.Backend");
     }
@@ -144,25 +144,31 @@ public class Folks.Backends.Sw.Backend : Folks.Backend
           return;
         }
 
-      this._prepare_pending = true;
-
-      foreach (var store in this._persona_stores.values)
+      try
         {
-          store.removed.disconnect (this.store_removed_cb);
-          this.persona_store_removed (store);
-        }
+          this._prepare_pending = true;
+
+          foreach (var store in this._persona_stores.values)
+            {
+              store.removed.disconnect (this.store_removed_cb);
+              this.persona_store_removed (store);
+            }
 
-      this._client = null;
+          this._client = null;
 
-      this._persona_stores.clear ();
-      this.notify_property ("persona-stores");
+          this._persona_stores.clear ();
+          this.notify_property ("persona-stores");
 
-      this._is_quiescent = false;
-      this.notify_property ("is-quiescent");
+          this._is_quiescent = false;
+          this.notify_property ("is-quiescent");
 
-      this._is_prepared = false;
-      this._prepare_pending = false;
-      this.notify_property ("is-prepared");
+          this._is_prepared = false;
+          this.notify_property ("is-prepared");
+        }
+      finally
+        {
+          this._prepare_pending = false;
+        }
     }
 
   private void add_service (string service_name)
diff --git a/backends/telepathy/lib/tpf-persona-store.vala b/backends/telepathy/lib/tpf-persona-store.vala
index fc6486d4..00915476 100644
--- a/backends/telepathy/lib/tpf-persona-store.vala
+++ b/backends/telepathy/lib/tpf-persona-store.vala
@@ -563,7 +563,6 @@ public class Tpf.PersonaStore : Folks.PersonaStore
               "(ID: %s)", this.id);
 
           this._is_prepared = true;
-          this._prepare_pending = false;
           this.notify_property ("is-prepared");
         }
       finally
diff --git a/backends/telepathy/tp-backend.vala b/backends/telepathy/tp-backend.vala
index d6acbae7..0734dbb8 100644
--- a/backends/telepathy/tp-backend.vala
+++ b/backends/telepathy/tp-backend.vala
@@ -137,6 +137,8 @@ public class Folks.Backends.Tp.Backend : Folks.Backend
 
       try
         {
+          this._prepare_pending = true;
+
           this._account_manager.account_enabled.disconnect (
               this._account_enabled_cb);
           this._account_manager.account_validity_changed.disconnect (
diff --git a/backends/tracker/lib/trf-persona-store.vala b/backends/tracker/lib/trf-persona-store.vala
index 437d0333..a95d8abb 100644
--- a/backends/tracker/lib/trf-persona-store.vala
+++ b/backends/tracker/lib/trf-persona-store.vala
@@ -1142,10 +1142,6 @@ public class Trf.PersonaStore : Folks.PersonaStore
               this.removed ();
               throw new PersonaStoreError.INVALID_ARGUMENT (e3.message);
             }
-          finally
-            {
-              this._prepare_pending = false;
-            }
         }
       finally
         {
@@ -1326,14 +1322,11 @@ public class Trf.PersonaStore : Folks.PersonaStore
           if (e.pred_id == rdf_type_id &&
               e.object_id == nco_person_id)
             {
-              lock (this._personas)
+              var removed_p = this._personas.get (p_id);
+              if (removed_p != null)
                 {
-                  var removed_p = this._personas.get (p_id);
-                  if (removed_p != null)
-                    {
-                      removed_personas.add (removed_p);
-                      _personas.unset (removed_p.iid);
-                    }
+                  removed_personas.add (removed_p);
+                  _personas.unset (removed_p.iid);
                 }
             }
           else
@@ -1363,15 +1356,12 @@ public class Trf.PersonaStore : Folks.PersonaStore
           var subject_tracker_id = e.subject_id.to_string ();
           var p_id = Trf.Persona.build_iid (this.id, subject_tracker_id);
           Trf.Persona persona;
-          lock (this._personas)
+          persona = this._personas.get (p_id);
+          if (persona == null)
             {
-              persona = this._personas.get (p_id);
-              if (persona == null)
-                {
-                  persona = new Trf.Persona (this, subject_tracker_id);
-                  this._personas.set (persona.iid, persona);
-                  added_personas.add (persona);
-                }
+              persona = new Trf.Persona (this, subject_tracker_id);
+              this._personas.set (persona.iid, persona);
+              added_personas.add (persona);
             }
           yield this._do_update (persona, e);
         }
@@ -1389,21 +1379,18 @@ public class Trf.PersonaStore : Folks.PersonaStore
       try {
         Sparql.Cursor cursor = yield this._connection.query_async (query);
 
-        lock (this._personas)
+        while (cursor.next ())
           {
-            while (cursor.next ())
+            int tracker_id =
+                (int) cursor.get_integer (Trf.Fields.TRACKER_ID);
+            var p_id =
+                Trf.Persona.build_iid (this.id, tracker_id.to_string ());
+            if (this._personas.get (p_id) == null)
               {
-                int tracker_id =
-                    (int) cursor.get_integer (Trf.Fields.TRACKER_ID);
-                var p_id =
-                    Trf.Persona.build_iid (this.id, tracker_id.to_string ());
-                if (this._personas.get (p_id) == null)
-                  {
-                    var persona = new Trf.Persona (this,
-                        tracker_id.to_string (), cursor);
-                    this._personas.set (persona.iid, persona);
-                    added_personas.add (persona);
-                  }
+                var persona = new Trf.Persona (this,
+                    tracker_id.to_string (), cursor);
+                this._personas.set (persona.iid, persona);
+                added_personas.add (persona);
               }
           }
 
diff --git a/tests/folks/async-locking.vala b/tests/folks/async-locking.vala
index 9957031f..fd424feb 100644
--- a/tests/folks/async-locking.vala
+++ b/tests/folks/async-locking.vala
@@ -9,6 +9,9 @@ public class AsyncLockingTests : Folks.TestCase
   int _total_calls;
   MainLoop _loop;
 
+  // This test was added to ensure removing lock (this._is_prepared) in prepare
+  // Methods is not going to cause a problem.  
+  // See bug: https://bugzilla.gnome.org/show_bug.cgi?id=652637
   public AsyncLockingTests ()
     {
       base ("AsyncLocking");