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).

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 aa3f05f5..83ffbb5a 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)
             {