Attachment 78018 Details for Bug 54972 – [1] DBusAtomic: on Unix, use pthreads mutexes for fallback

[patch] [1] DBusAtomic: on Unix, use pthreads mutexes for fallback

0001-DBusAtomic-on-Unix-use-pthreads-mutexes-for-fallback.patch (text/plain), 4.07 KB, created by Simon McVittie on 2013-04-15 20:34:42 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-04-15 20:34:42 UTC

Size: 4.07 KB

patch

obsolete

>From cf42b69a1d1eb11d48208b6b638baba39449fc97 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Mon, 15 Apr 2013 13:51:19 +0100
>Subject: [PATCH 1/8] DBusAtomic: on Unix, use pthreads mutexes for fallback
>
>On pthreads platforms, POSIX guarantees that we can "allocate" mutexes
>as library-global variables, without involving malloc. This means we
>don't need to error-check their allocation - if the dynamic linker
>succeeds, then we have enough memory for all our globals - which is an
>important step towards being thread-safe by default. In particular,
>making atomic operations never rely on DBusMutex means that we are free
>to implement parts of DBusMutex in terms of DBusAtomic, if it would help.
>
>We do not currently support any non-Windows platform that does not have
>pthreads. This is unlikely to change.
>
>On Windows, we already used real atomic operations; we can just
>delete the unused global variable.
>
>Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
>---
> dbus/dbus-internals.h    |    5 -----
> dbus/dbus-sysdeps-unix.c |   27 ++++++++++++++++++++-------
> dbus/dbus-sysdeps-win.c  |    2 --
> dbus/dbus-threads.c      |    3 ---
> 4 files changed, 20 insertions(+), 17 deletions(-)
>
>diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h
>index 80376ad..28486a8 100644
>--- a/dbus/dbus-internals.h
>+++ b/dbus/dbus-internals.h
>@@ -323,12 +323,7 @@ _DBUS_DECLARE_GLOBAL_LOCK (win_fds);
> _DBUS_DECLARE_GLOBAL_LOCK (sid_atom_cache);
> _DBUS_DECLARE_GLOBAL_LOCK (machine_uuid);
> 
>-#if !DBUS_USE_SYNC
>-_DBUS_DECLARE_GLOBAL_LOCK (atomic);
>-#define _DBUS_N_GLOBAL_LOCKS (15)
>-#else
> #define _DBUS_N_GLOBAL_LOCKS (14)
>-#endif
> 
> dbus_bool_t _dbus_threads_init_debug (void);
> 
>diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
>index 88db415..34dfbe7 100644
>--- a/dbus/dbus-sysdeps-unix.c
>+++ b/dbus/dbus-sysdeps-unix.c
>@@ -82,6 +82,10 @@
> 
> #include "sd-daemon.h"
> 
>+#if !DBUS_USE_SYNC
>+#include <pthread.h>
>+#endif
>+
> #ifndef O_BINARY
> #define O_BINARY 0
> #endif
>@@ -2428,7 +2432,12 @@ _dbus_parse_uid (const DBusString      *uid_str,
> }
> 
> #if !DBUS_USE_SYNC
>-_DBUS_DEFINE_GLOBAL_LOCK (atomic);
>+/* To be thread-safe by default on platforms that don't necessarily have
>+ * atomic operations (notably Debian armel, which is armv4t), we must
>+ * use a mutex that can be initialized statically, like this.
>+ * GLib >= 2.32 uses a similar system.
>+ */
>+static pthread_mutex_t atomic_mutex = PTHREAD_MUTEX_INITIALIZER;
> #endif
> 
> /**
>@@ -2444,10 +2453,12 @@ _dbus_atomic_inc (DBusAtomic *atomic)
>   return __sync_add_and_fetch(&atomic->value, 1)-1;
> #else
>   dbus_int32_t res;
>-  _DBUS_LOCK (atomic);
>+
>+  pthread_mutex_lock (&atomic_mutex);
>   res = atomic->value;
>   atomic->value += 1;
>-  _DBUS_UNLOCK (atomic);
>+  pthread_mutex_unlock (&atomic_mutex);
>+
>   return res;
> #endif
> }
>@@ -2466,10 +2477,11 @@ _dbus_atomic_dec (DBusAtomic *atomic)
> #else
>   dbus_int32_t res;
> 
>-  _DBUS_LOCK (atomic);
>+  pthread_mutex_lock (&atomic_mutex);
>   res = atomic->value;
>   atomic->value -= 1;
>-  _DBUS_UNLOCK (atomic);
>+  pthread_mutex_unlock (&atomic_mutex);
>+
>   return res;
> #endif
> }
>@@ -2490,9 +2502,10 @@ _dbus_atomic_get (DBusAtomic *atomic)
> #else
>   dbus_int32_t res;
> 
>-  _DBUS_LOCK (atomic);
>+  pthread_mutex_lock (&atomic_mutex);
>   res = atomic->value;
>-  _DBUS_UNLOCK (atomic);
>+  pthread_mutex_unlock (&atomic_mutex);
>+
>   return res;
> #endif
> }
>diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
>index 66d6b76..4a54981 100644
>--- a/dbus/dbus-sysdeps-win.c
>+++ b/dbus/dbus-sysdeps-win.c
>@@ -3188,8 +3188,6 @@ _dbus_get_standard_system_servicedirs (DBusList **dirs)
>   return TRUE;
> }
> 
>-_DBUS_DEFINE_GLOBAL_LOCK (atomic);
>-
> /**
>  * Atomically increments an integer
>  *
>diff --git a/dbus/dbus-threads.c b/dbus/dbus-threads.c
>index bb1169d..b464629 100644
>--- a/dbus/dbus-threads.c
>+++ b/dbus/dbus-threads.c
>@@ -496,9 +496,6 @@ init_locks (void)
>     LOCK_ADDR (pending_call_slots),
>     LOCK_ADDR (server_slots),
>     LOCK_ADDR (message_slots),
>-#if !DBUS_USE_SYNC
>-    LOCK_ADDR (atomic),
>-#endif
>     LOCK_ADDR (bus),
>     LOCK_ADDR (bus_datas),
>     LOCK_ADDR (shutdown_funcs),
>-- 
>1.7.10.4
>

Actions: View | Splinter Review

Attachments on bug 54972: 78018 | 78019 | 78020 | 78021 | 78022 | 78023 | 78024 | 78025 | 78086 | 78087 | 78088 | 78089 | 78091 | 78093 | 78094 | 78095 | 78096 | 78097 | 78137 | 79028

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.