[glib/wip/gcleanup: 11/71] WIP cleanup for qdata

[Stefan Walter <stefw src gnome org>]

commit 6f5c379d5902d2f008a8fc7c936d9e2d6175c25a
Author: Stef Walter <stefw gnome org>
Date:   Wed Nov 13 08:57:09 2013 +0100

    WIP cleanup for qdata

 glib/gcleanup.c     |   43 +++++++++++++++++++++++
 glib/gcleanup.h     |    3 ++
 glib/gdataset.c     |   93 ++++++++++++++++++++++++++++++++++++++++++--------
 glib/gdataset.h     |   21 +++++++++++
 glib/glib-private.h |    2 +
 5 files changed, 147 insertions(+), 15 deletions(-)
---
diff --git a/glib/gcleanup.c b/glib/gcleanup.c
index 797d08f..e6295a4 100644
--- a/glib/gcleanup.c
+++ b/glib/gcleanup.c
@@ -568,6 +568,49 @@ g_cleanup_clean (GCleanupScope *scope)
     fprintf (stderr, "GLib-Cleanup-DEBUG: cleanup: done\n");
 }
 
+static gint qdata_table_lock = 0;
+static GHashTable *qdata_table = NULL;
+
+void
+g_cleanup_push_qdata (GCleanupScope *cleanup,
+                      GQuark         qdata)
+{
+  g_bit_lock (&qdata_table_lock, 1);
+
+  if (cleanup)
+    {
+      if (!qdata_table)
+        {
+          qdata_table = g_hash_table_new (g_direct_hash, g_direct_equal);
+
+          /* This table gets cleaned up in the glib scope, after all other stuff */
+          g_cleanup_push (G_CLEANUP_SCOPE, 1020, (GCleanupFunc)g_hash_table_unref, qdata_table);
+        }
+
+      g_hash_table_insert (qdata_table, GUINT_TO_POINTER (qdata), cleanup);
+    }
+  else if (qdata_table)
+    {
+      g_hash_table_remove (qdata_table, GUINT_TO_POINTER (qdata));
+    }
+
+  g_bit_unlock (&qdata_table_lock, 1);
+}
+
+GCleanupScope *
+g_cleanup_scope_for_qdata (GQuark qdata)
+{
+  GCleanupScope *scope = NULL;
+
+  g_bit_lock (&qdata_table_lock, 1);
+
+  if (qdata_table)
+    scope = g_hash_table_lookup (qdata_table, GUINT_TO_POINTER (qdata));
+
+  g_bit_unlock (&qdata_table_lock, 1);
+
+  return scope;
+}
 /**
  * G_CLEANUP_DEFINE:
  *
diff --git a/glib/gcleanup.h b/glib/gcleanup.h
index f52ef9b..c250d61 100644
--- a/glib/gcleanup.h
+++ b/glib/gcleanup.h
@@ -72,6 +72,9 @@ GLIB_AVAILABLE_IN_2_40
 void                    g_cleanup_push_source            (GCleanupScope *cleanup,
                                                           gint           phase,
                                                           GSource       *source);
+GLIB_AVAILABLE_IN_2_40
+void                    g_cleanup_push_qdata             (GCleanupScope *cleanup,
+                                                          GQuark         qdata);
 GLIB_AVAILABLE_IN_2_40 /* NOTE: annotate() very useful for debugging, but might not merge */
 void                    g_cleanup_annotate               (gpointer       cleanup_item,
                                                           const gchar   *func_name);
diff --git a/glib/gdataset.c b/glib/gdataset.c
index 006bdc1..7818550 100644
--- a/glib/gdataset.c
+++ b/glib/gdataset.c
@@ -37,6 +37,9 @@
 #include "gdataset.h"
 #include "gbitlock.h"
 
+#include "glib-private.h"
+
+#include "gcleanup.h"
 #include "gslice.h"
 #include "gdatasetprivate.h"
 #include "ghash.h"
@@ -153,11 +156,16 @@
   } while (!g_atomic_pointer_compare_and_exchange ((void**) datalist, _oldv, _newv));   \
 } G_STMT_END
 
+enum {
+  DESTROY_IS_CLEANUP = 1 << 0
+};
+
 /* --- structures --- */
 typedef struct {
   GQuark          key;
+  gint            flags;
   gpointer        data;
-  GDestroyNotify  destroy;
+  gpointer        destroy;
 } GDataElt;
 
 typedef struct _GDataset GDataset;
@@ -219,6 +227,30 @@ g_datalist_unlock (GData **datalist)
   g_pointer_bit_unlock ((void **)datalist, DATALIST_LOCK_BIT);
 }
 
+static inline void
+invoke_destroy_notify (GDataElt *el)
+{
+  GDestroyNotify func;
+
+  if (el->flags & DESTROY_IS_CLEANUP)
+    func = g_cleanup_steal (el->destroy, NULL);
+  else
+    func = el->destroy;
+
+  g_assert (func != NULL);
+  func (el->data);
+}
+
+static inline void
+steal_destroy_notify (GDataElt *el)
+{
+  if (el->flags & DESTROY_IS_CLEANUP)
+    {
+      g_cleanup_steal (el->destroy, NULL);
+      el->destroy = NULL;
+    }
+}
+
 /* Called with the datalist lock held, or the dataset global
  * lock for dataset lists
  */
@@ -237,7 +269,7 @@ g_datalist_clear_i (GData **datalist)
       for (i = 0; i < data->len; i++)
         {
           if (data->data[i].data && data->data[i].destroy)
-            data->data[i].destroy (data->data[i].data);
+            invoke_destroy_notify (data->data + i);
         }
       G_LOCK (g_dataset_global);
 
@@ -274,7 +306,7 @@ g_datalist_clear (GData **datalist)
       for (i = 0; i < data->len; i++)
         {
           if (data->data[i].data && data->data[i].destroy)
-            data->data[i].destroy (data->data[i].data);
+            invoke_destroy_notify (data->data + i);
         }
 
       g_free (data);
@@ -354,6 +386,8 @@ g_data_set_internal (GData    **datalist,
 {
   GData *d, *old_d;
   GDataElt old, *data, *data_last, *data_end;
+  GCleanupScope *cleanup;
+  gint flags = 0;
 
   g_datalist_lock (datalist);
 
@@ -404,15 +438,22 @@ g_data_set_internal (GData          **datalist,
                   * a special hint combination to "steal"
                   * data without destroy notification
                   */
-                 if (old.destroy && !new_destroy_func)
-                   {
-                     if (dataset)
-                       G_UNLOCK (g_dataset_global);
-                     old.destroy (old.data);
-                     if (dataset)
-                       G_LOCK (g_dataset_global);
-                     old.data = NULL;
-                   }
+                  if (old.destroy)
+                      {
+                        if (!new_destroy_func)
+                          {
+                            if (dataset)
+                              G_UNLOCK (g_dataset_global);
+                            invoke_destroy_notify (&old);
+                            if (dataset)
+                              G_UNLOCK (g_dataset_global);
+                            old.data = NULL;
+                          }
+                        else
+                          {
+                            steal_destroy_notify (&old);
+                          }
+                      }
 
                  return old.data;
                }
@@ -422,6 +463,10 @@ g_data_set_internal (GData   **datalist,
     }
   else
     {
+      cleanup = g_cleanup_scope_for_qdata (key_id);
+      if (cleanup)
+        flags |= DESTROY_IS_CLEANUP;
+
       old.data = NULL;
       if (d)
        {
@@ -431,16 +476,24 @@ g_data_set_internal (GData          **datalist,
            {
              if (data->key == key_id)
                {
+                  data->flags = flags;
+                  data->data = new_data;
+
+                  if (cleanup)
+                    {
+                      new_destroy_func = g_cleanup_push_pointer (cleanup,
+                                                                 G_CLEANUP_PHASE_LATE,
+                                                                 new_destroy_func, &data->data);
+                      g_cleanup_annotate (new_destroy_func, g_quark_to_string (key_id));
+                    }
                  if (!data->destroy)
                    {
-                     data->data = new_data;
                      data->destroy = new_destroy_func;
                      g_datalist_unlock (datalist);
                    }
                  else
                    {
                      old = *data;
-                     data->data = new_data;
                      data->destroy = new_destroy_func;
 
                      g_datalist_unlock (datalist);
@@ -451,7 +504,7 @@ g_data_set_internal (GData    **datalist,
                       */
                      if (dataset)
                        G_UNLOCK (g_dataset_global);
-                     old.destroy (old.data);
+                      invoke_destroy_notify (&old);
                      if (dataset)
                        G_LOCK (g_dataset_global);
                    }
@@ -477,8 +530,18 @@ g_data_set_internal (GData   **datalist,
       if (old_d != d)
        G_DATALIST_SET_POINTER (datalist, d);
 
+      d->data[d->len].flags = flags;
       d->data[d->len].key = key_id;
       d->data[d->len].data = new_data;
+
+      if (cleanup)
+        {
+          new_destroy_func = g_cleanup_push_pointer (cleanup,
+                                                     G_CLEANUP_PHASE_LATE,
+                                                     new_destroy_func, &d->data[d->len].data);
+          g_cleanup_annotate (new_destroy_func, g_quark_to_string (key_id));
+        }
+
       d->data[d->len].destroy = new_destroy_func;
       d->len++;
     }
diff --git a/glib/gdataset.h b/glib/gdataset.h
index 6d69e60..e63851f 100644
--- a/glib/gdataset.h
+++ b/glib/gdataset.h
@@ -47,6 +47,9 @@ GLIB_AVAILABLE_IN_ALL
 void     g_datalist_init                (GData            **datalist);
 GLIB_AVAILABLE_IN_ALL
 void     g_datalist_clear               (GData            **datalist);
+#if 0
+xxxx needs to not clear data if in cleanup mode xxxx
+#endif
 GLIB_AVAILABLE_IN_ALL
 gpointer g_datalist_id_get_data         (GData            **datalist,
                                         GQuark             key_id);
@@ -55,6 +58,14 @@ void     g_datalist_id_set_data_full    (GData            **datalist,
                                         GQuark             key_id,
                                         gpointer           data,
                                         GDestroyNotify     destroy_func);
+#if 0
+GLIB_AVAILABLE_IN_ALL
+void     g_datalist_id_set_data_cleanup (GData            **datalist,
+                                         GQuark             key_id,
+                                         gpointer           data,
+                                         GDestroyNotify     destroy_func);
+xxxx specify cleanup xxxx
+#endif
 
 typedef gpointer (*GDuplicateFunc) (gpointer data, gpointer user_data);
 
@@ -70,6 +81,16 @@ gboolean g_datalist_id_replace_data     (GData            **datalist,
                                          gpointer           newval,
                                          GDestroyNotify     destroy,
                                         GDestroyNotify    *old_destroy);
+#if 0
+GLIB_AVAILABLE_IN_2_34
+gboolean g_datalist_id_replace_cleanup  (GData            **datalist,
+                                         GQuark             key_id,
+                                         gpointer           oldval,
+                                         gpointer           newval,
+                                         GDestroyNotify     destroy,
+                                         GDestroyNotify    *old_destroy);
+xxxx specify cleanup xxxxx
+#endif
 
 GLIB_AVAILABLE_IN_ALL
 gpointer g_datalist_id_remove_no_notify (GData            **datalist,
diff --git a/glib/glib-private.h b/glib/glib-private.h
index 44cbf3d..9cb7d0f 100644
--- a/glib/glib-private.h
+++ b/glib/glib-private.h
@@ -36,6 +36,8 @@ gchar *_glib_get_locale_dir    (void);
 GDir * g_dir_open_with_errno (const gchar *path, guint flags);
 GDir * g_dir_new_from_dirp (gpointer dirp);
 
+GCleanupScope *         g_cleanup_scope_for_qdata       (GQuark qdata);
+
 #define GLIB_PRIVATE_CALL(symbol) (glib__private__()->symbol)
 
 typedef struct {