Stop dithering over GPrivate

Take out the half-private g_private_init() stuff and replace it with a
G_PRIVATE_INIT macro that allows specifying a GDestroyNotify.

Expose the GPrivate structure in a public header.

Add a g_private_replace() to (sort of) match the functionality of
g_static_mutex_set().

Improve the documentation.

Deprecate g_private_new().

docs/reference/glib/glib-overrides.txt

@@ -172,24 +172,10 @@ GCond *cond
 </FUNCTION>
 
 # GPrivate
-
+<MACRO>
-<FUNCTION>
+<NAME>G_PRIVATE_INIT</NAME>
-<NAME>g_private_new</NAME>
+#define G_PRIVATE_INIT(notify)
-<RETURNS>GPrivate*</RETURNS>
+</MACRO>
-GDestroyNotify destructor
-</FUNCTION>
-
-<FUNCTION>
-<NAME>g_private_get</NAME>
-<RETURNS>gpointer</RETURNS>
-GPrivate *private_key
-</FUNCTION>
-
-<FUNCTION>
-<NAME>g_private_set</NAME>
-<RETURNS>void</RETURNS>
-GPrivate *private_key, gpointer data
-</FUNCTION>
 
 # GStaticPrivate
 

docs/reference/glib/glib-sections.txt

@@ -691,9 +691,13 @@ g_cond_broadcast
 
 <SUBSECTION>
 GPrivate
-g_private_new
+G_PRIVATE_INIT
 g_private_get
 g_private_set
+g_private_replace
+
+<SUBSECTION>
+g_private_new
 
 <SUBSECTION>
 GStaticPrivate

glib/deprecated/gthread-deprecated.c

@@ -24,6 +24,7 @@
 #include "config.h"
 
 #include "gmessages.h"
+#include "gslice.h"
 #include "gmain.h"
 #include "gthread.h"
 #include "gthreadprivate.h"
@@ -973,5 +974,28 @@ g_static_rw_lock_free (GStaticRWLock* lock)
   g_static_mutex_free (&lock->mutex);
 }
 
-/* vim: set foldmethod=marker: */
+/* GPrivate {{{1 ------------------------------------------------------ */
 
+/**
+ * g_private_new:
+ * @notify: a #GDestroyNotify
+ *
+ * Deprecated:2.32: dynamic allocation of #GPrivate is a bad idea.  Use
+ *                  static storage and G_PRIVATE_INIT() instead.
+ *
+ * Returns: a newly allocated #GPrivate (which can never be destroyed)
+ */
+GPrivate *
+g_private_new (GDestroyNotify notify)
+{
+  GPrivate tmp = G_PRIVATE_INIT (notify);
+  GPrivate *key;
+
+  key = g_slice_new (GPrivate);
+  *key = tmp;
+
+  return key;
+}
+
+/* Epilogue {{{1 */
+/* vim: set foldmethod=marker: */

glib/deprecated/gthread.h

@@ -175,6 +175,9 @@ gboolean  g_static_rw_lock_writer_trylock (GStaticRWLock* lock);
 void      g_static_rw_lock_writer_unlock  (GStaticRWLock* lock);
 void      g_static_rw_lock_free           (GStaticRWLock* lock);
 
+
+GPrivate *      g_private_new             (GDestroyNotify  notify);
+
 G_END_DECLS
 
 #endif /* __G_DEPRECATED_THREAD_H__ */

glib/glib.symbols

@@ -1617,6 +1617,7 @@ g_mutex_trylock
 g_mutex_unlock
 g_private_new
 g_private_get
+g_private_replace
 g_private_set
 g_rw_lock_clear
 g_rw_lock_init

glib/gmessages.c

@@ -67,7 +67,6 @@
 #include "gprintfint.h"
 #include "gtestutils.h"
 #include "gthread.h"
-#include "gthreadprivate.h"
 #include "gstrfuncs.h"
 #include "gstring.h"
 
@@ -108,7 +107,6 @@ static GLogLevelFlags g_log_always_fatal = G_LOG_FATAL_MASK;
 static GPrintFunc     glib_print_func = NULL;
 static GPrintFunc     glib_printerr_func = NULL;
 static GPrivate       g_log_depth;
-static gboolean       g_log_depth_initialised;
 static GLogLevelFlags g_log_msg_prefix = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_DEBUG;
 static GLogFunc       default_log_func = g_log_default_handler;
 static gpointer       default_log_data = NULL;
@@ -527,11 +525,6 @@ g_logv (const gchar   *log_domain,
 
 	  /* check recursion and lookup handler */
 	  g_mutex_lock (&g_messages_lock);
-          if (!g_log_depth_initialised)
-            {
-              g_private_init (&g_log_depth, NULL);
-              g_log_depth_initialised = TRUE;
-            }
           depth = GPOINTER_TO_UINT (g_private_get (&g_log_depth));
 	  domain = g_log_find_domain_L (log_domain ? log_domain : "");
 	  if (depth)

glib/gslice.c

@@ -50,7 +50,6 @@
 #include "gutils.h"
 #include "gtestutils.h"
 #include "gthread.h"
-#include "gthreadprivate.h"
 #include "glib_trace.h"
 #include "glib-ctor.h"
 
@@ -203,7 +202,7 @@ static int      smc_notify_free   (void   *pointer,
                                    size_t  size);
 
 /* --- variables --- */
-static GPrivate    private_thread_memory;
+static GPrivate    private_thread_memory = G_PRIVATE_INIT (private_thread_memory_cleanup);
 static gsize       sys_page_size = 0;
 static Allocator   allocator[1] = { { 0, }, };
 static SliceConfig slice_config = {
@@ -360,7 +359,6 @@ GLIB_CTOR (g_slice_init_nomessage)
   /* at this point, g_mem_gc_friendly() should be initialized, this
    * should have been accomplished by the above g_malloc/g_new calls
    */
-  g_private_init (&private_thread_memory, private_thread_memory_cleanup);
 }
 
 static inline guint

glib/gthread-posix.c

@@ -757,53 +757,158 @@ g_cond_timedwait (GCond  *cond,
 
 /* {{{1 GPrivate */
 
-void
+/**
-g_private_init (GPrivate       *key,
+ * GPrivate:
-                GDestroyNotify  notify)
+ *
+ * The #GPrivate struct is an opaque data structure to represent a
+ * thread-local data key.  It is approximately equivalent to the
+ * <function>pthread_setspecific()</function>/<function>pthread_getspecific()</function>
+ * APIs on POSIX and to
+ * <function>TlsSetValue()</function>/<function>TlsGetValue<()/function> on
+ * Windows.
+ *
+ * If you don't already know why you might want this functionality, then
+ * you probably don't need it.
+ *
+ * #GPrivate is a very limited resource (as far as 128 per program,
+ * shared between all libraries).  It is also not possible to destroy a
+ * #GPrivate after it has been used. As such, it is only ever acceptable
+ * to use #GPrivate in static scope, and even then sparingly so.
+ *
+ * See G_PRIVATE_INIT() for a couple of examples.
+ *
+ * The #GPrivate structure should be considered opaque.  It should only
+ * be accessed via the <function>g_private_</function> functions.
+ */
+
+/**
+ * G_PRIVATE_INIT:
+ * @notify: a #GDestroyNotify
+ *
+ * A macro to assist with the static initialisation of a #GPrivate.
+ *
+ * This macro is useful for the case that a #GDestroyNotify function
+ * should be associated the key.  This is needed when the key will be
+ * used to point at memory that should be deallocated when the thread
+ * exits.
+ *
+ * Additionally, the #GDestroyNotify will also be called on the previous
+ * value stored in the key when g_private_replace() is used.
+ *
+ * If no #GDestroyNotify is needed, then use of this macro is not
+ * required -- if the #GPrivate is declared in static scope then it will
+ * be properly initialised by default (ie: to all zeros).  See the
+ * examples below.
+ *
+ * |[
+ * static GPrivate name_key = G_PRIVATE_INIT (g_free);
+ *
+ * // return value should not be freed
+ * const gchar *
+ * get_local_name (void)
+ * {
+ *   return g_private_get (&name_key);
+ * }
+ *
+ * void
+ * set_local_name (const gchar *name)
+ * {
+ *   g_private_replace (&name_key, g_strdup (name));
+ * }
+ *
+ *
+ * static GPrivate count_key;   // no free function
+ *
+ * gint
+ * get_local_count (void)
+ * {
+ *   return GPOINTER_TO_INT (g_private_get (&count_key));
+ * }
+ *
+ * void
+ * set_local_count (gint count)
+ * {
+ *   g_private_set (&count_key, GINT_TO_POINTER (count));
+ * }
+ * ]|
+ *
+ * Since: 2.32
+ **/
+
+static pthread_key_t *
+g_private_impl_new (GDestroyNotify notify)
 {
-  pthread_key_create (&key->key, notify);
+  pthread_key_t *key;
-  key->ready = TRUE;
+  gint status;
+
+  key = malloc (sizeof (pthread_key_t));
+  if G_UNLIKELY (key == NULL)
+    g_thread_abort (errno, "malloc");
+  status = pthread_key_create (key, notify);
+  if G_UNLIKELY (status != 0)
+    g_thread_abort (status, "pthread_key_create");
+
+  return key;
+}
+
+static void
+g_private_impl_free (pthread_key_t *key)
+{
+  gint status;
+
+  status = pthread_key_delete (*key);
+  if G_UNLIKELY (status != 0)
+    g_thread_abort (status, "pthread_key_delete");
+  free (key);
+}
+
+static pthread_key_t *
+g_private_get_impl (GPrivate *key)
+{
+  pthread_key_t *impl = key->p;
+
+  if G_UNLIKELY (impl == NULL)
+    {
+      impl = g_private_impl_new (key->notify);
+      if (!g_atomic_pointer_compare_and_exchange (&key->p, NULL, impl))
+        {
+          g_private_impl_free (impl);
+          impl = key->p;
+        }
+    }
+
+  return impl;
 }
 
 /**
  * g_private_get:
- * @private_key: a #GPrivate
+ * @key: a #GPrivate
  *
- * Returns the pointer keyed to @private_key for the current thread. If
+ * Returns the current value of the thread local variable @key.
- * g_private_set() hasn't been called for the current @private_key and
- * thread yet, this pointer will be %NULL.
  *
- * This function can be used even if g_thread_init() has not yet been
+ * If the value has not yet been set in this thread, %NULL is returned.
- * called, and, in that case, will return the value of @private_key
+ * Values are never copied between threads (when a new thread is
- * casted to #gpointer. Note however, that private data set
+ * created, for example).
- * <emphasis>before</emphasis> g_thread_init() will
- * <emphasis>not</emphasis> be retained <emphasis>after</emphasis> the
- * call. Instead, %NULL will be returned in all threads directly after
- * g_thread_init(), regardless of any g_private_set() calls issued
- * before threading system initialization.
  *
- * Returns: the corresponding pointer
+ * Returns: the thread-local value
  */
 gpointer
 g_private_get (GPrivate *key)
 {
-  if (!key->ready)
-    return key->single_value;
-
   /* quote POSIX: No errors are returned from pthread_getspecific(). */
-  return pthread_getspecific (key->key);
+  return pthread_getspecific (*g_private_get_impl (key));
 }
 
 /**
  * g_private_set:
- * @private_key: a #GPrivate
+ * @key: a #GPrivate
- * @data: the new pointer
+ * @value: the new value
  *
- * Sets the pointer keyed to @private_key for the current thread.
+ * Sets the thread local variable @key to have the value @value in the
+ * current thread.
  *
- * This function can be used even if g_thread_init() has not yet been
+ * This function differs from g_private_replace() in the following way:
- * called, and, in that case, will set @private_key to @data casted to
+ * the #GDestroyNotify for @key is not called on the old value.
- * #GPrivate*. See g_private_get() for resulting caveats.
  */
 void
 g_private_set (GPrivate *key,
@@ -811,13 +916,37 @@ g_private_set (GPrivate *key,
 {
   gint status;
 
-  if (!key->ready)
+  if G_UNLIKELY ((status = pthread_setspecific (*g_private_get_impl (key), value)) != 0)
-    {
+    g_thread_abort (status, "pthread_setspecific");
-      key->single_value = value;
+}
-      return;
-    }
 
-  if G_UNLIKELY ((status = pthread_setspecific (key->key, value)) != 0)
+/**
+ * g_private_replace:
+ * @key: a #GPrivate
+ * @value: the new value
+ *
+ * Sets the thread local variable @key to have the value @value in the
+ * current thread.
+ *
+ * This function differs from g_private_set() in the following way: if
+ * the previous value was non-%NULL then the #GDestroyNotify handler for
+ * @key is run on it.
+ *
+ * Since: 2.32
+ **/
+void
+g_private_replace (GPrivate *key,
+                   gpointer  value)
+{
+  pthread_key_t *impl = g_private_get_impl (key);
+  gpointer old;
+  gint status;
+
+  old = pthread_getspecific (*impl);
+  if (old && key->notify)
+    key->notify (old);
+
+  if G_UNLIKELY ((status = pthread_setspecific (*impl, value)) != 0)
     g_thread_abort (status, "pthread_setspecific");
 }
 

glib/gthread-win32.c

@@ -366,48 +366,80 @@ struct _GPrivateDestructor
 };
 
 static GPrivateDestructor * volatile g_private_destructors;
+static CRITICAL_SECTION g_private_lock;
 
-void
+static DWORD
-g_private_init (GPrivate       *key,
+g_private_get_impl (GPrivate *key)
-                GDestroyNotify  notify)
 {
-  GPrivateDestructor *destructor;
+  DWORD impl = (DWORD) key->p;
 
-  key->index = TlsAlloc ();
+  if G_UNLIKELY (impl == 0)
+    {
+      EnterCriticalSection (&g_private_lock);
+      impl = (DWORD) key->p;
+      if (impl == 0)
+        {
+          GPrivateDestructor *destructor;
 
-  destructor = malloc (sizeof (GPrivateDestructor));
+          impl = TlsAlloc ();
-  if G_UNLIKELY (destructor == NULL)
-    g_thread_abort (errno, "malloc");
-  destructor->index = key->index;
-  destructor->notify = notify;
 
-  do
+          if (impl == TLS_OUT_OF_INDEXES)
-    destructor->next = g_private_destructors;
+            g_thread_abort (0, "TlsAlloc");
-  while (InterlockedCompareExchangePointer (&g_private_destructors, destructor->next, destructor) != destructor->next);
 
-  key->ready = TRUE;
+          if (key->notify != NULL)
+            {
+              destructor = malloc (sizeof (GPrivateDestructor));
+              if G_UNLIKELY (destructor == NULL)
+                g_thread_abort (errno, "malloc");
+              destructor->index = impl;
+              destructor->notify = key->notify;
+              destructor->next = g_private_destructors;
+
+              /* We need to do an atomic store due to the unlocked
+               * access to the destructor list from the thread exit
+               * function.
+               *
+               * It can double as a sanity check...
+               */
+              if (InterlockedCompareExchangePointer (&g_private_destructors, destructor,
+                                                     destructor->next) != destructor->next)
+                g_thread_abort (0, "g_private_get_impl(1)");
+            }
+
+          /* Ditto, due to the unlocked access on the fast path */
+          if (InterlockedCompareExchangePointer (&key->p, impl, NULL) != NULL)
+            g_thread_abort (0, "g_private_get_impl(2)");
+        }
+      LeaveCriticalSection (&g_private_lock);
+    }
+
+  return impl;
 }
 
 gpointer
 g_private_get (GPrivate *key)
 {
-  if (!key->ready)
+  return TlsGetValue (g_private_get_impl (key));
-    return key->single_value;
-
-  return TlsGetValue (key->index);
 }
 
 void
 g_private_set (GPrivate *key,
                gpointer  value)
 {
-  if (!key->ready)
+  TlsSetValue (g_private_get_impl (key), value);
-    {
+}
-      key->single_value = value;
-      return;
-    }
 
-  TlsSetValue (key->index, value);
+void
+g_private_replace (GPrivate *key,
+                   gpointer  value)
+{
+  DWORD impl = g_private_get_impl (key);
+  gpointer old;
+
+  old = TlsGetValue (impl, value);
+  if (old && key->notify)
+    key->notify (old);
+  TlsSetValue (impl, value);
 }
 
 /* {{{1 GThread */
@@ -425,7 +457,6 @@ g_private_set (GPrivate *key,
 #define G_MUTEX_SIZE (sizeof (gpointer))
 
 static DWORD g_thread_self_tls;
-static DWORD g_private_tls;
 
 typedef BOOL (__stdcall *GTryEnterCriticalSectionFunc) (CRITICAL_SECTION *);
 
@@ -1051,7 +1082,7 @@ g_thread_DllMain (void)
     }
 
   win32_check_for_error (TLS_OUT_OF_INDEXES != (g_thread_self_tls = TlsAlloc ()));
-  win32_check_for_error (TLS_OUT_OF_INDEXES != (g_private_tls = TlsAlloc ()));
+  InitializeCriticalSection (&g_private_lock);
 }
 
 /* vim:set foldmethod=marker: */

glib/gthread.c

@@ -523,58 +523,6 @@
  * Since: 2.32
  */
 
-/* GPrivate Documentation {{{1 --------------------------------------- */
-
-/**
- * GPrivate:
- *
- * <note><para>
- * #GStaticPrivate is a better choice for most uses.
- * </para></note>
- *
- * The #GPrivate struct is an opaque data structure to represent a
- * thread private data key. Threads can thereby obtain and set a
- * pointer which is private to the current thread. Take our
- * <function>give_me_next_number(<!-- -->)</function> example from
- * above.  Suppose we don't want <literal>current_number</literal> to be
- * shared between the threads, but instead to be private to each thread.
- * This can be done as follows:
- *
- * <example>
- *  <title>Using GPrivate for per-thread data</title>
- *  <programlisting>
- *   GPrivate* current_number_key = NULL; /<!-- -->* Must be initialized somewhere
- *                                           with g_private_new (g_free); *<!-- -->/
- *
- *   int
- *   give_me_next_number (void)
- *   {
- *     int *current_number = g_private_get (current_number_key);
- *
- *     if (!current_number)
- *       {
- *         current_number = g_new (int, 1);
- *         *current_number = 0;
- *         g_private_set (current_number_key, current_number);
- *       }
- *
- *     *current_number = calc_next_number (*current_number);
- *
- *     return *current_number;
- *   }
- *  </programlisting>
- * </example>
- *
- * Here the pointer belonging to the key
- * <literal>current_number_key</literal> is read. If it is %NULL, it has
- * not been set yet. Then get memory for an integer value, assign this
- * memory to the pointer and write the pointer back. Now we have an
- * integer value that is private to the current thread.
- *
- * The #GPrivate struct should only be accessed via the
- * <function>g_private_</function> functions.
- */
-
 /* GThread Documentation {{{1 ---------------------------------------- */
 
 /**
@@ -649,7 +597,8 @@ GMutex           g_once_mutex = G_MUTEX_INIT;
 static GCond     g_once_cond = G_COND_INIT;
 static GSList   *g_once_init_list = NULL;
 
-static GPrivate     g_thread_specific_private;
+static void g_thread_cleanup (gpointer data);
+static GPrivate     g_thread_specific_private = G_PRIVATE_INIT (g_thread_cleanup);
 static GRealThread *g_thread_all_threads = NULL;
 static GSList      *g_thread_free_indices = NULL;
 
@@ -684,8 +633,6 @@ G_LOCK_DEFINE_STATIC (g_thread);
  * having to link with the thread libraries.</para></note>
  */
 
-static void g_thread_cleanup (gpointer data);
-
 void
 g_thread_init_glib (void)
 {
@@ -704,7 +651,6 @@ g_thread_init_glib (void)
 
   /* setup the basic threading system */
   g_threads_got_initialized = TRUE;
-  g_private_init (&g_thread_specific_private, g_thread_cleanup);
   g_private_set (&g_thread_specific_private, main_thread);
   g_system_thread_self (&main_thread->system_thread);
 
@@ -1537,43 +1483,5 @@ g_cond_free (GCond *cond)
   g_slice_free (GCond, cond);
 }
 
-/* GPrivate {{{1 ------------------------------------------------------ */
+/* Epilogue {{{1 */
-
+/* vim: set foldmethod=marker: */
-/**
- * g_private_new:
- * @destructor: a function to destroy the data keyed to
- *     the #GPrivate when a thread ends
- *
- * Creates a new #GPrivate. If @destructor is non-%NULL, it is a
- * pointer to a destructor function. Whenever a thread ends and the
- * corresponding pointer keyed to this instance of #GPrivate is
- * non-%NULL, the destructor is called with this pointer as the
- * argument.
- *
- * <note><para>
- * #GStaticPrivate is a better choice for most uses.
- * </para></note>
- *
- * <note><para>@destructor is used quite differently from @notify in
- * g_static_private_set().</para></note>
- *
- * <note><para>A #GPrivate cannot be freed. Reuse it instead, if you
- * can, to avoid shortage, or use #GStaticPrivate.</para></note>
- *
- * <note><para>This function will abort if g_thread_init() has not been
- * called yet.</para></note>
- *
- * Returns: a newly allocated #GPrivate
- */
-GPrivate *
-g_private_new (GDestroyNotify notify)
-{
-  GPrivate *key;
-
-  key = g_slice_new (GPrivate);
-  g_private_init (key, notify);
-
-  return key;
-}
-
- /* vim: set foldmethod=marker: */

glib/gthread.h

@@ -112,6 +112,14 @@ struct _GRecMutex
   gpointer impl;
 };
 
+#define G_PRIVATE_INIT(notify) { NULL, (notify), { NULL, NULL } }
+struct _GPrivate
+{
+  gpointer       p;
+  GDestroyNotify notify;
+  gpointer future[2];
+};
+
 void     g_thread_init   (gpointer vtable);
 
 gboolean g_thread_get_initialized (void);
@@ -274,10 +282,11 @@ gboolean                g_cond_timedwait                                (GCond
                                                                          GMutex         *mutex,
                                                                          gint64          abs_time);
 
-GPrivate *              g_private_new                                   (GDestroyNotify  notify);
 gpointer                g_private_get                                   (GPrivate       *key);
 void                    g_private_set                                   (GPrivate       *key,
                                                                          gpointer        value);
+void                    g_private_replace                               (GPrivate       *key,
+                                                                         gpointer        value);
 
 G_END_DECLS
 

glib/gthreadprivate.h

@@ -56,20 +56,6 @@ void g_thread_init_glib (void);
 /* initializers that may also use g_private_new() */
 G_GNUC_INTERNAL void _g_messages_thread_init_nomessage      (void);
 
-struct _GPrivate
-{
-  gpointer single_value;
-  gboolean ready;
-#ifdef G_OS_WIN32
-  gint index;
-#else
-  pthread_key_t key;
-#endif
-};
-
-G_GNUC_INTERNAL void g_private_init (GPrivate       *key,
-                                     GDestroyNotify  notify);
-
 #ifdef G_OS_WIN32
 G_GNUC_INTERNAL void g_thread_DllMain (void);
 #endif