Replace individual global-lock variables with an array of DBusRMutex *

This means we can use a much simpler code structure in data-slot
allocators: instead of giving them a DBusRMutex ** at first-allocation,
we can just give them an index into the array, which can be done
statically.

It doesn't make us any more thread-safe-by-default - the mutexes will
only actually be used if threads were already initialized - but it's
substantially better than nothing.

These locks really do have to be recursive: for instance,
internal_bus_get() calls dbus_bus_register() under the bus lock,
and dbus_bus_register() can call _dbus_connection_close_possibly_shared(),
which takes the bus lock.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=54972
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Alban Crequy <alban.crequy@collabora.co.uk>
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Reviewed-by: Anas Nashif <anas.nashif@intel.com>

1 files changed, 45 insertions, 55 deletions

diff --git a/dbus/dbus-threads.c b/dbus/dbus-threads.c
index 43676bc1..297a7e4e 100644
--- a/dbus/dbus-threads.c
+++ b/dbus/dbus-threads.c
@@ -343,23 +343,19 @@ _dbus_condvar_wake_one (DBusCondVar *cond)
     _dbus_platform_condvar_wake_one (cond);
 }
 
+static DBusRMutex *global_locks[_DBUS_N_GLOBAL_LOCKS] = { NULL };
+
 static void
-shutdown_global_locks (void *data)
+shutdown_global_locks (void *nil)
 {
-  DBusRMutex ***locks = data;
   int i;
 
-  i = 0;
-  while (i < _DBUS_N_GLOBAL_LOCKS)
+  for (i = 0; i < _DBUS_N_GLOBAL_LOCKS; i++)
     {
-      if (*(locks[i]) != NULL)
-        _dbus_platform_rmutex_free (*(locks[i]));
-
-      *(locks[i]) = NULL;
-      ++i;
+      _dbus_assert (global_locks[i] != NULL);
+      _dbus_platform_rmutex_free (global_locks[i]);
+      global_locks[i] = NULL;
     }
-  
-  dbus_free (locks);
 }
 
 static void
@@ -483,67 +479,60 @@ init_uninitialized_locks (void)
 }
 
 static dbus_bool_t
-init_locks (void)
+init_global_locks (void)
 {
   int i;
-  DBusRMutex ***dynamic_global_locks;
-  DBusRMutex **global_locks[] = {
-#define LOCK_ADDR(name) (& _dbus_lock_##name)
-    LOCK_ADDR (list),
-    LOCK_ADDR (connection_slots),
-    LOCK_ADDR (pending_call_slots),
-    LOCK_ADDR (server_slots),
-    LOCK_ADDR (message_slots),
-    LOCK_ADDR (bus),
-    LOCK_ADDR (bus_datas),
-    LOCK_ADDR (shutdown_funcs),
-    LOCK_ADDR (system_users),
-    LOCK_ADDR (message_cache),
-    LOCK_ADDR (shared_connections),
-    LOCK_ADDR (machine_uuid)
-#undef LOCK_ADDR
-  };
-
-  _DBUS_STATIC_ASSERT (_DBUS_N_ELEMENTS (global_locks) == _DBUS_N_GLOBAL_LOCKS);
-
-  i = 0;
-  
-  dynamic_global_locks = dbus_new (DBusRMutex**, _DBUS_N_GLOBAL_LOCKS);
-  if (dynamic_global_locks == NULL)
-    goto failed;
-  
-  while (i < _DBUS_N_ELEMENTS (global_locks))
+  dbus_bool_t ok;
+
+  for (i = 0; i < _DBUS_N_GLOBAL_LOCKS; i++)
     {
-      *global_locks[i] = _dbus_platform_rmutex_new ();
+      _dbus_assert (global_locks[i] == NULL);
+
+      global_locks[i] = _dbus_platform_rmutex_new ();
 
-      if (*global_locks[i] == NULL)
+      if (global_locks[i] == NULL)
         goto failed;
+    }
 
-      dynamic_global_locks[i] = global_locks[i];
+  _dbus_lock (_DBUS_LOCK_NAME (shutdown_funcs));
+  ok = _dbus_register_shutdown_func_unlocked (shutdown_global_locks, NULL);
+  _dbus_unlock (_DBUS_LOCK_NAME (shutdown_funcs));
 
-      ++i;
-    }
-  
-  if (!_dbus_register_shutdown_func (shutdown_global_locks,
-                                     dynamic_global_locks))
+  if (!ok)
     goto failed;
 
-  if (!init_uninitialized_locks ())
-    goto failed;
-  
   return TRUE;
 
  failed:
-  dbus_free (dynamic_global_locks);
-                                     
   for (i = i - 1; i >= 0; i--)
     {
-      _dbus_platform_rmutex_free (*global_locks[i]);
-      *global_locks[i] = NULL;
+      _dbus_platform_rmutex_free (global_locks[i]);
+      global_locks[i] = NULL;
     }
+
   return FALSE;
 }
 
+void
+_dbus_lock (DBusGlobalLock lock)
+{
+  _dbus_assert (lock >= 0);
+  _dbus_assert (lock < _DBUS_N_GLOBAL_LOCKS);
+
+  if (thread_init_generation == _dbus_current_generation)
+    _dbus_platform_rmutex_lock (global_locks[lock]);
+}
+
+void
+_dbus_unlock (DBusGlobalLock lock)
+{
+  _dbus_assert (lock >= 0);
+  _dbus_assert (lock < _DBUS_N_GLOBAL_LOCKS);
+
+  if (thread_init_generation == _dbus_current_generation)
+    _dbus_platform_rmutex_unlock (global_locks[lock]);
+}
+
 /** @} */ /* end of internals */
 
 /**
@@ -587,7 +576,8 @@ dbus_threads_init (const DBusThreadFunctions *functions)
     }
 
   if (!_dbus_threads_init_platform_specific() ||
-      !init_locks ())
+      !init_global_locks () ||
+      !init_uninitialized_locks ())
     {
       _dbus_threads_unlock_platform_specific ();
       return FALSE;