[glib] GDBusConnection: replace is_initialized with an atomic flag

[Simon McVittie <smcv src gnome org>]

commit 245d68be6ff0104783ce0b2d4bc0a139f09e0c34
Author: Simon McVittie <simon mcvittie collabora co uk>
Date:   Fri Oct 21 16:02:22 2011 +0100

    GDBusConnection: replace is_initialized with an atomic flag
    
    The comment implied that even failed initialization would set
    is_initialized = TRUE, but this wasn't the case - failed initialization
    would only set initialization_error, and it was necessary to check both.
    
    It turns out the documented semantics are nicer than the implemented
    semantics, since this lets us use atomic operations, which are also
    memory barriers, to avoid needing separate memory barriers or locks
    for initialization_error (and other members that are read-only after
    construction).
    
    I expect to need more than one atomically-accessed flag to fix thread
    safety, so instead of a minimal implementation I've turned is_initialized
    into a flags word.
    
    Signed-off-by: Simon McVittie <simon mcvittie collabora co uk>
    Bug: https://bugzilla.gnome.org/show_bug.cgi?id=661689
    Bug: https://bugzilla.gnome.org/show_bug.cgi?id=661992
    Reviewed-by: David Zeuthen <davidz redhat com>

 gio/gdbusconnection.c |   44 +++++++++++++++++++++++++++++---------------
 1 files changed, 29 insertions(+), 15 deletions(-)
---
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
index 8a1ea15..a509bf4 100644
--- a/gio/gdbusconnection.c
+++ b/gio/gdbusconnection.c
@@ -300,6 +300,11 @@ _g_strv_has_string (const gchar* const *haystack,
     g_mutex_unlock (&(obj)->lock);                                      \
   } while (FALSE)
 
+/* Flags in connection->atomic_flags */
+enum {
+    FLAG_INITIALIZED = 1 << 0
+};
+
 /**
  * GDBusConnection:
  *
@@ -355,10 +360,16 @@ struct _GDBusConnection
    */
   gchar *guid;
 
-  /* set to TRUE exactly when initable_init() has finished running */
-  gboolean is_initialized;
+  /* FLAG_INITIALIZED is set exactly when initable_init() has finished running.
+   * Inspect @initialization_error to see whether it succeeded or failed.
+   */
+  volatile gint atomic_flags;
 
-  /* If the connection could not be established during initable_init(), this GError will set */
+  /* If the connection could not be established during initable_init(),
+   * this GError will be set.
+   * Read-only after initable_init(), so it may be read if you either
+   * hold @init_lock or check for initialization first.
+   */
   GError *initialization_error;
 
   /* The result of g_main_context_ref_thread_default() when the object
@@ -635,7 +646,14 @@ g_dbus_connection_real_closed (GDBusConnection *connection,
                                gboolean         remote_peer_vanished,
                                GError          *error)
 {
-  if (remote_peer_vanished && connection->exit_on_close && connection->is_initialized)
+  gint flags = g_atomic_int_get (&connection->atomic_flags);
+
+  /* Because atomic int access is a memory barrier, we can safely read
+   * initialization_error without a lock, as long as we do it afterwards.
+   */
+  if (remote_peer_vanished && connection->exit_on_close &&
+      (flags & FLAG_INITIALIZED) != 0 &&
+      connection->initialization_error == NULL)
     {
       if (error != NULL)
         {
@@ -2309,20 +2327,17 @@ initable_init (GInitable     *initable,
 
   ret = FALSE;
 
-  /* First, handle the case where the connection already has an
-   * initialization error set.
+  /* Make this a no-op if we're already initialized (successfully or
+   * unsuccessfully)
    */
-  if (connection->initialization_error != NULL)
-    goto out;
-
-  /* Also make this a no-op if we're already initialized fine */
-  if (connection->is_initialized)
+  if ((g_atomic_int_get (&connection->atomic_flags) & FLAG_INITIALIZED))
     {
-      ret = TRUE;
+      ret = (connection->initialization_error == NULL);
       goto out;
     }
 
-  g_assert (connection->initialization_error == NULL && !connection->is_initialized);
+  /* Because of init_lock, we can't get here twice in different threads */
+  g_assert (connection->initialization_error == NULL);
 
   /* The user can pass multiple (but mutally exclusive) construct
    * properties:
@@ -2462,8 +2477,6 @@ initable_init (GInitable     *initable,
       //g_debug ("unique name is `%s'", connection->bus_unique_name);
     }
 
-  connection->is_initialized = TRUE;
-
   ret = TRUE;
  out:
   if (!ret)
@@ -2472,6 +2485,7 @@ initable_init (GInitable     *initable,
       g_propagate_error (error, g_error_copy (connection->initialization_error));
     }
 
+  g_atomic_int_or (&connection->atomic_flags, FLAG_INITIALIZED);
   g_mutex_unlock (&connection->init_lock);
 
   return ret;