[folks] eds: Eliminate another race condition when committing contacts



commit b5070cec811e68f5b8f8029119a059d3c3eb2ce9
Author: Philip Withnall <philip withnall collabora co uk>
Date:   Tue Jun 16 17:58:15 2015 +0100

    eds: Eliminate another race condition when committing contacts
    
    We can’t connect to the objects_modified signal, as we can’t guarantee
    whether the handler installed in _commit_modified_property() or the
    _contacts_changed_cb() handler will be called first. Since they both
    enqueue to the idle queue in order, the callback from
    _commit_modified_property() will still operate on an outdated
    Edsf.Persona if it is called first.
    
    This race condition can only happen when _commit_modified_property() is
    called with a null property_name, which happens for custom properties
    and extended fields.
    
    Fix this by listening for changes to the Edsf.Persona.contact property
    instead of objects_modified. This is always guaranteed to change when a
    Persona is updated (because it represents the current vCard), and fixes
    the ordering problem, because the Persona’s properties will have been
    updated by the time the contact property is notified (see
    Edsf.Persona._update() for details).

 backends/eds/lib/edsf-persona-store.vala |   61 ++++-------------------------
 1 files changed, 9 insertions(+), 52 deletions(-)
---
diff --git a/backends/eds/lib/edsf-persona-store.vala b/backends/eds/lib/edsf-persona-store.vala
index aa3f05f..83ffbb5 100644
--- a/backends/eds/lib/edsf-persona-store.vala
+++ b/backends/eds/lib/edsf-persona-store.vala
@@ -1362,57 +1362,18 @@ public class Edsf.PersonaStore : Folks.PersonaStore
         {
           var received_notification = false;
           var has_yielded = false;
+          var signal_name = property_name ?? "contact";
 
-          if (property_name != null)
+          signal_id = persona.notify[signal_name].connect ((obj, pspec) =>
             {
-              signal_id = persona.notify[property_name].connect ((obj, pspec) =>
-                {
-                  /* Success! Return to _commit_modified_property(). */
-                  received_notification = true;
+              /* Success! Return to _commit_modified_property(). */
+              received_notification = true;
 
-                  if (has_yielded == true)
-                    {
-                      this._commit_modified_property.callback ();
-                    }
-                });
-            }
-          else
-            {
-              signal_id = ((!) this._ebookview).objects_modified.connect (
-                  (_contacts) =>
+              if (has_yielded == true)
                 {
-                  unowned GLib.List<E.Contact> contacts =
-                      (GLib.List<E.Contact>) _contacts;
-
-                  /* Ignore other personas. */
-                  foreach (E.Contact c in contacts)
-                    {
-                      var iid = Edsf.Persona.build_iid_from_contact (this.id, c);
-                      if (iid != persona.iid)
-                        {
-                          continue;
-                        }
-
-                      /* Success! Return to _commit_modified_property(), but do
-                       * it via the idle queue so that the notification is
-                       * queued after the actual contact updates in
-                       * _contacts_changed_idle(). */
-                      this._idle_queue (() =>
-                        {
-                          received_notification = true;
-
-                          if (has_yielded == true)
-                            {
-                              this._commit_modified_property.callback ();
-                            }
-
-                          return false;
-                        });
-
-                      return;
-                    }
-                });
-            }
+                  this._commit_modified_property.callback ();
+                }
+            });
 
           /* Commit the modification. _addressbook is asserted as being non-null
            * above. */
@@ -1466,14 +1427,10 @@ public class Edsf.PersonaStore : Folks.PersonaStore
       finally
         {
           /* Remove the callbacks. */
-          if (signal_id != 0 && property_name != null)
+          if (signal_id != 0)
             {
               persona.disconnect (signal_id);
             }
-          else if (signal_id != 0)
-            {
-              ((!) this._ebookview).disconnect (signal_id);
-            }
 
           if (timeout_id != 0)
             {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]