[folks] telepathy: Fix a race condition in the handling of weak refs to TpContacts



commit 00590b87e2316c7dbd74a072ece72c30ffd89354
Author: Philip Withnall <philip tecnocode co uk>
Date:   Sun Sep 2 20:56:44 2012 +0100

    telepathy: Fix a race condition in the handling of weak refs to TpContacts
    
    Use a WeakRef to maintain the pointer to a TpContact in Tpf.Persona,
    rather than g_object_weak_[un]ref() and manually nullifying the pointer.
    
    See the comments in bug #680335 for a full explanation.
    
    Closes: https://bugzilla.gnome.org/show_bug.cgi?id=680335

 NEWS                                    |    2 +
 backends/telepathy/lib/tpf-persona.vala |   34 +++++++++++++++++++++++-------
 configure.ac                            |    2 +-
 3 files changed, 29 insertions(+), 9 deletions(-)
---
diff --git a/NEWS b/NEWS
index 1f3be2c..7cccf6b 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,8 @@ Bugs fixed:
 â Bug 682719 â eds test fails to compile
 â Bug 683319 â Crash in individual-aggregator: _personas_changed_cb
 â Bug 681164 â Folks-inspect linking fails
+â Bug 680335 â empathy crashed with SIGSEGV in
+  _tpf_persona_contact_weak_notify_cb()
 
 Overview of changes from libfolks 0.7.2 to libfolks 0.7.3
 =========================================================
diff --git a/backends/telepathy/lib/tpf-persona.vala b/backends/telepathy/lib/tpf-persona.vala
index 4c46dc4..58118bb 100644
--- a/backends/telepathy/lib/tpf-persona.vala
+++ b/backends/telepathy/lib/tpf-persona.vala
@@ -562,14 +562,22 @@ public class Tpf.Persona : Folks.Persona,
     }
 
   /* This has to be weak since, in general, we can't force any TpContacts to
-   * remain alive if we want to solve bgo#665376. */
-  private weak Contact? _contact = null;
+   * remain alive if we want to solve bgo#665376.
+   * As per bgo#680335, we have to use a WeakRef rather than a
+   * âweak Contact?â to avoid races when clearing the pointer. We still have
+   * to use a weak ref. notifier as well, though, in order to be able to emit
+   * a property change notification for ::contact.
+   *
+   * FIXME: Once bgo#554344 is fixed, _contact could be changed back to
+   * being a 'weak Contact?', assuming Vala implements weak references using
+   * GWeakRef. */
+  private WeakRef _contact = WeakRef (null);
 
   private void _contact_weak_notify_cb (Object obj)
     {
       debug ("TpContact %p destroyed; setting ._contact = null in Persona %p",
           obj, this);
-      this._contact = null;
+      /* _contact is cleared automatically as it's a WeakRef. */
       this.notify_property ("contact");
     }
 
@@ -586,12 +594,21 @@ public class Tpf.Persona : Folks.Persona,
     {
       get
         {
-          if (this._contact == null)
+          /* FIXME: This property should be changed to transfer its reference
+           * when the API is next broken. This is necessary because the
+           * TpfPersona doesn't hold a strong ref to the TpContact, so any
+           * pointer which is returned might be invalidated before reaching the
+           * caller. Probably not a problem in practice since folks won't be
+           * run multi-threaded. */
+          Contact? contact = (Contact?) this._contact.get ();
+          if (contact == null)
             {
               return null;
             }
 
-          return this._contact;
+          /* FIXME: I'm so very, very sorry. This is to cause Vala to forget
+           * we have a strong ref on 'contact' and not transfer it out. */
+          return (Contact) ((void*) contact);
         }
 
       construct
@@ -601,7 +618,7 @@ public class Tpf.Persona : Folks.Persona,
               value.weak_ref (this._contact_weak_notify_cb);
             }
 
-          this._contact = value;
+          this._contact.set (value);
         }
     }
 
@@ -1163,9 +1180,10 @@ public class Tpf.Persona : Folks.Persona,
     {
       debug ("Destroying Tpf.Persona '%s': %p", this.uid, this);
 
-      if (this._contact != null)
+      var contact = this._contact.get ();
+      if (contact != null)
         {
-          this._contact.weak_unref (this._contact_weak_notify_cb);
+          contact.weak_unref (this._contact_weak_notify_cb);
         }
     }
 
diff --git a/configure.ac b/configure.ac
index fc7ab65..4216457 100644
--- a/configure.ac
+++ b/configure.ac
@@ -158,7 +158,7 @@ AM_CONDITIONAL([ENABLE_LIBSOCIALWEB],
 # Dependencies
 # -----------------------------------------------------------
 
-GLIB_REQUIRED=2.26.0
+GLIB_REQUIRED=2.32.0
 VALA_REQUIRED=0.15.2
 VALADOC_REQUIRED=0.3.1
 TRACKER_SPARQL_MAJOR=0.14



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