[glibmm] Glib::Source: Fix the destruction and deletion.

[Kjell Ahlstedt <kjellahl src gnome org>]

commit 746f8ba0259cb88c0c6b667f4a663766fcfba359
Author: Kjell Ahlstedt <kjell ahlstedt bredband net>
Date:   Thu Apr 18 15:47:19 2013 +0200

    Glib::Source: Fix the destruction and deletion.
    
    * glib/glibmm/main.cc: Keep the Glib::Source wrapper until the GSource
    instance has been destroyed and all RefPtr<Source> have been deleted.
    Keep a separate ref count in glibmm. Bug #561885.

 ChangeLog           |  8 ++++++
 glib/glibmm/main.cc | 74 ++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 73 insertions(+), 9 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 4be1dca..9ebbea0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2013-04-18  Kjell Ahlstedt  <kjell ahlstedt bredband net>
+
+       Glib::Source: Fix the destruction and deletion.
+
+       * glib/glibmm/main.cc: Keep the Glib::Source wrapper until the GSource
+       instance has been destroyed and all RefPtr<Source> have been deleted.
+       Keep a separate ref count in glibmm. Bug #561885.
+
 2013-04-14  José Alburquerque  <jaalburquerque gmail com>
 
        ByteArray: get_data(): Add a const version.
diff --git a/glib/glibmm/main.cc b/glib/glibmm/main.cc
index e48756d..173f831 100644
--- a/glib/glibmm/main.cc
+++ b/glib/glibmm/main.cc
@@ -32,6 +32,7 @@
 #include <glibmm/wrap.h>
 #include <glibmm/iochannel.h>
 #include <algorithm>
+#include <map> // Needed until the next ABI break.
 
 namespace
 {
@@ -46,6 +47,27 @@ void time64_to_time_val(gint64 time64, Glib::TimeVal& time_val)
 }
 #endif //GLIBMM_DISABLE_DEPRECATED
 
+//TODO: At the next ABI break, replace ExtraSourceData by new data members in Source.
+// Then the mutex is not necessary, but to keep the code thread-safe, use the
+// g_atomic_*() functions on these data elements.
+// These are new data members that can't be added to Glib::Source now,
+// because it would break ABI.
+struct ExtraSourceData
+{
+  ExtraSourceData()
+  : ref_count(1), keep_wrapper(2)
+  {}
+  int ref_count;
+  // When both Source::unreference() and SourceCallbackData::destroy_notify_callback()
+  // have decreased keep_wrapper, it's time to delete the C++ wrapper.
+  int keep_wrapper;
+};
+
+std::map<const Glib::Source*, ExtraSourceData> extra_source_data;
+// Source instances may be used in different threads.
+// Accesses to extra_source_data must be thread-safe.
+Glib::Threads::Mutex extra_source_data_mutex;
+
 class SourceConnectionNode
 {
 public:
@@ -120,8 +142,8 @@ sigc::slot_base* SourceConnectionNode::get_slot()
 /* We use the callback data member of GSource to store both a pointer to our
  * wrapper and a pointer to the connection node that is currently being used.
  * The one and only SourceCallbackData object of a Glib::Source is constructed
- * in the ctor of Glib::Source and destroyed after the GSource object when the
- * reference counter of the GSource object reaches zero!
+ * in the ctor of Glib::Source and destroyed when the GSource object is destroyed,
+ * which may occur before the reference counter of the GSource object reaches zero!
  */
 struct SourceCallbackData
 {
@@ -159,7 +181,16 @@ void SourceCallbackData::destroy_notify_callback(void* data)
     SourceConnectionNode::destroy_notify_callback(self->node);
 
   if(self->wrapper)
-    Glib::Source::destroy_notify_callback(self->wrapper);
+  {
+    Glib::Threads::Mutex::Lock lock(extra_source_data_mutex);
+    if (--extra_source_data[self->wrapper].keep_wrapper == 0)
+    {
+      // No other reference exists to the wrapper. Delete it!
+      extra_source_data.erase(self->wrapper);
+      lock.release();
+      Glib::Source::destroy_notify_callback(self->wrapper);
+    }
+  }
 
   delete self;
 }
@@ -169,7 +200,7 @@ void SourceCallbackData::destroy_notify_callback(void* data)
  */
 static SourceCallbackData* glibmm_source_get_callback_data(GSource* source)
 {
-  g_return_val_if_fail(source->callback_funcs->get != 0, 0);
+  g_return_val_if_fail(source->callback_funcs != 0, 0);
 
   GSourceFunc func;
   void* user_data = 0;
@@ -194,9 +225,9 @@ static gboolean glibmm_dummy_source_callback(void*)
   return 0;
 }
 
-/* Only used by SignalTimeout::connect() and SignalIdle::connect().
- * These don't use Glib::Source, to avoid the unnecessary overhead
- * of a completely unused wrapper object.
+/* Only used by SignalTimeout::connect(), SignalTimeout::connect_seconds()
+ * and SignalIdle::connect(). These don't use Glib::Source, to avoid the
+ * unnecessary overhead of a completely unused wrapper object.
  */
 static gboolean glibmm_source_callback(void* data)
 {
@@ -821,12 +852,35 @@ GSource* Source::gobj_copy() const
 
 void Source::reference() const
 {
-  g_source_ref(gobject_);
+  Glib::Threads::Mutex::Lock lock(extra_source_data_mutex);
+  ++extra_source_data[this].ref_count;
 }
 
 void Source::unreference() const
 {
-  g_source_unref(gobject_);
+  Glib::Threads::Mutex::Lock lock(extra_source_data_mutex);
+  if (--extra_source_data[this].ref_count == 0)
+  {
+    GSource* const tmp_gobject = gobject_;
+
+    if (--extra_source_data[this].keep_wrapper == 0)
+    {
+      // The last reference from a RefPtr<Source> has been deleted, and
+      // SourceCallbackData::destroy_notify_callback() has been called while
+      // extra_source_data[this].keep_wrapper was > 1.
+      // Delete the wrapper!
+      extra_source_data.erase(this);
+      lock.release();
+      destroy_notify_callback(const_cast<Source*>(this));
+    }
+    else
+      lock.release();
+
+    // Drop the one and only GSource reference held by the C++ wrapper.
+    // If the GSource instance is attached to a main context, the GMainContext
+    // holds a reference until the source is detached (destroyed).
+    g_source_unref(tmp_gobject);
+  }
 }
 
 Source::Source()
@@ -864,6 +918,8 @@ Source::~Source()
     gobject_ = 0;
 
     g_source_unref(tmp_gobject);
+
+    // The constructor does not add this to extra_source_data. No need to erase.
   }
 }