[telepathy-glib-0.22] telepathy-glib: TpContact: avoid a race condition in avatar handling

[Simon McVittie]

Module: telepathy-glib
Branch: telepathy-glib-0.22
Commit: fcfa0973c74c9d03020b6ea8c42a0362ccff791e
URL:    http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=fcfa0973c74c9d03020b6ea8c42a0362ccff791e

Author: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date:   Tue Oct  1 17:08:00 2013 +0100

TpContact: avoid a race condition in avatar handling

We have to remember the avatar token synchronously, before we start
async-saving the cached file, so that we can't get into this
situation:

    my avatar is token "A", bytes "AAAA..."
    my avatar changes to token "B", bytes "BBBB..."
    we start saving the file /.../B
    my avatar changes to token "C", bytes "CCCC..."
    saving the file /.../B finishes
    change-notification announces that my avatar has changed to "B"

which is particularly problematic for Mission Control.

Regression in 0.21.2 (fd.o #63402).

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=70010
Reviewed-by: Guillaume Desmottes <guillaume.desmottes at collabora.co.uk>

---

 telepathy-glib/contact.c |   36 +++++++++++++++++++++++++++++++-----
 1 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/telepathy-glib/contact.c b/telepathy-glib/contact.c
index abf38ef..4a77f11 100644
--- a/telepathy-glib/contact.c
+++ b/telepathy-glib/contact.c
@@ -40,6 +40,15 @@
 #include "telepathy-glib/util-internal.h"
 #include "telepathy-glib/variant-util-internal.h"
 
+static const gchar *
+nonnull (const gchar *s)
+{
+  if (s == NULL)
+    return "(null)";
+
+  return s;
+}
+
 /**
  * SECTION:contact
  * @title: TpContact
@@ -2805,18 +2814,28 @@ mime_file_written (GObject *source_object,
 
   self = g_weak_ref_get (&avatar_data->contact);
 
-  if (self != NULL)
+  if (self == NULL)
+    {
+      DEBUG ("No relevant TpContact");
+    }
+  else if (tp_strdiff (avatar_data->token, self->priv->avatar_token))
     {
+      DEBUG ("Contact's avatar token has changed from %s to %s, "
+          "this avatar is no longer relevant",
+          avatar_data->token, nonnull (self->priv->avatar_token));
+    }
+  else
+    {
+      DEBUG ("Saved avatar '%s' of MIME type '%s' still used by '%s' to '%s'",
+          avatar_data->token, avatar_data->mime_type,
+          self->priv->identifier,
+          g_file_get_path (avatar_data->file));
       g_clear_object (&self->priv->avatar_file);
       self->priv->avatar_file = g_object_ref (avatar_data->file);
 
       g_free (self->priv->avatar_mime_type);
       self->priv->avatar_mime_type = g_strdup (avatar_data->mime_type);
 
-      /* Update the avatar token if a newer one is given
-       * (this emits notify::avatar-token if needed) */
-      contact_set_avatar_token (self, avatar_data->token, FALSE);
-
       /* Notify both property changes together once both files have been
        * written */
       g_object_notify ((GObject *) self, "avatar-mime-type");
@@ -2872,6 +2891,13 @@ contact_avatar_retrieved (TpConnection *connection,
   gchar *mime_filename;
   WriteAvatarData *avatar_data;
 
+  if (self != NULL)
+    {
+      /* Update the avatar token if a newer one is given
+       * (this emits notify::avatar-token if needed) */
+      contact_set_avatar_token (self, token, FALSE);
+    }
+
   if (!build_avatar_filename (connection, token, TRUE, &filename,
       &mime_filename))
     return;