Attachment 78086 Details for Bug 54972 – dbus_threads_init_default, dbus_threads_init: be safe to call at any time

[patch] dbus_threads_init_default, dbus_threads_init: be safe to call at any time

0001-dbus_threads_init_default-dbus_threads_init-be-safe-.patch (text/plain), 8.39 KB, created by Simon McVittie on 2013-04-16 14:06:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-04-16 14:06:33 UTC

Size: 8.39 KB

patch

obsolete

>From cb4b3a6be01d05f11b339fdb91340177ee71f10a Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Tue, 16 Apr 2013 12:07:23 +0100
>Subject: [PATCH] dbus_threads_init_default, dbus_threads_init: be safe to
> call at any time
>
>On Unix, we use a pthreads mutex, which can be allocated and
>initialized in global memory.
>
>On Windows, we use a CRITICAL_SECTION, together with a call to
>InitializeCriticalSection() from the constructor of a global static
>C++ object (thanks to Ralf Habacker for suggesting this approach).
>---
> cmake/dbus/CMakeLists.txt      |    1 +
> dbus/Makefile.am               |    1 +
> dbus/dbus-init-win.cpp         |   44 ++++++++++++++++++++++++++++++++++++++++
> dbus/dbus-memory.c             |    7 +++++++
> dbus/dbus-sysdeps-pthread.c    |   14 +++++++++++++
> dbus/dbus-sysdeps-thread-win.c |   24 ++++++++++++++++++++++
> dbus/dbus-sysdeps-win.h        |    2 ++
> dbus/dbus-sysdeps.h            |   12 +++++++++++
> dbus/dbus-threads.c            |   28 ++++++++++++++++++-------
> 9 files changed, 126 insertions(+), 7 deletions(-)
> create mode 100644 dbus/dbus-init-win.cpp
>
>diff --git a/cmake/dbus/CMakeLists.txt b/cmake/dbus/CMakeLists.txt
>index 66772c5..ce61e1a 100644
>--- a/cmake/dbus/CMakeLists.txt
>+++ b/cmake/dbus/CMakeLists.txt
>@@ -186,6 +186,7 @@ set (DBUS_UTIL_HEADERS
> if (WIN32)
> 	set (DBUS_SHARED_SOURCES ${DBUS_SHARED_SOURCES} 
> 		${DBUS_DIR}/dbus-file-win.c
>+		${DBUS_DIR}/dbus-init-win.cpp
> 		${DBUS_DIR}/dbus-sysdeps-win.c
> 		${DBUS_DIR}/dbus-pipe-win.c
> 		${DBUS_DIR}/dbus-sysdeps-thread-win.c
>diff --git a/dbus/Makefile.am b/dbus/Makefile.am
>index 3679e3f..a9f35ed 100644
>--- a/dbus/Makefile.am
>+++ b/dbus/Makefile.am
>@@ -68,6 +68,7 @@ endif
> DBUS_SHARED_arch_sources = 			\
> 	$(wince_source)				\
> 	dbus-file-win.c 			\
>+	dbus-init-win.cpp			\
> 	dbus-pipe-win.c 			\
> 	dbus-sockets-win.h			\
> 	dbus-sysdeps-win.c 			\
>diff --git a/dbus/dbus-init-win.cpp b/dbus/dbus-init-win.cpp
>new file mode 100644
>index 0000000..f222d77
>--- /dev/null
>+++ b/dbus/dbus-init-win.cpp
>@@ -0,0 +1,44 @@
>+/*
>+ * dbus-init-win.cpp - once-per-process initialization
>+ *
>+ * Copyright Â© 2013 Intel Corporation
>+ *
>+ * Licensed under the Academic Free License version 2.1
>+ *
>+ * This program is free software; you can redistribute it and/or modify
>+ * it under the terms of the GNU General Public License as published by
>+ * the Free Software Foundation; either version 2 of the License, or
>+ * (at your option) any later version.
>+ *
>+ * This program is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+ * GNU General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU General Public License
>+ * along with this program; if not, write to the Free Software
>+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
>+ *
>+ */
>+
>+#include <config.h>
>+
>+extern "C"
>+{
>+#include "dbus-sysdeps-win.h"
>+}
>+
>+namespace
>+{
>+
>+  class Init
>+    {
>+      public:
>+        Init ()
>+          {
>+            _dbus_threads_windows_init_global ();
>+          }
>+    };
>+
>+  Init init;
>+}
>diff --git a/dbus/dbus-memory.c b/dbus/dbus-memory.c
>index a033b54..317e37e 100644
>--- a/dbus/dbus-memory.c
>+++ b/dbus/dbus-memory.c
>@@ -26,6 +26,7 @@
> #include "dbus-internals.h"
> #include "dbus-sysdeps.h"
> #include "dbus-list.h"
>+#include "dbus-threads.h"
> #include <stdlib.h>
> 
> /**
>@@ -890,7 +891,13 @@ dbus_shutdown (void)
>       dbus_free (c);
>     }
> 
>+  /* We wrap this in the thread-initialization lock because
>+   * dbus_threads_init() uses the current generation to tell whether
>+   * we're initialized, so we need to make sure that un-initializing
>+   * propagates into all threads. */
>+  _dbus_threads_lock_platform_specific ();
>   _dbus_current_generation += 1;
>+  _dbus_threads_unlock_platform_specific ();
> }
> 
> /** @} */ /** End of public API docs block */
>diff --git a/dbus/dbus-sysdeps-pthread.c b/dbus/dbus-sysdeps-pthread.c
>index 1344074..1300ec3 100644
>--- a/dbus/dbus-sysdeps-pthread.c
>+++ b/dbus/dbus-sysdeps-pthread.c
>@@ -286,3 +286,17 @@ _dbus_threads_init_platform_specific (void)
> 
>   return TRUE;
> }
>+
>+static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
>+
>+void
>+_dbus_threads_lock_platform_specific (void)
>+{
>+  pthread_mutex_lock (&init_mutex);
>+}
>+
>+void
>+_dbus_threads_unlock_platform_specific (void)
>+{
>+  pthread_mutex_unlock (&init_mutex);
>+}
>diff --git a/dbus/dbus-sysdeps-thread-win.c b/dbus/dbus-sysdeps-thread-win.c
>index 4c4442a..4b90ec2 100644
>--- a/dbus/dbus-sysdeps-thread-win.c
>+++ b/dbus/dbus-sysdeps-thread-win.c
>@@ -30,6 +30,17 @@
> 
> #include <windows.h>
> 
>+static dbus_bool_t global_init_done = FALSE;
>+static CRITICAL_SECTION init_lock;
>+
>+/* Called from C++ code in dbus-init-win.cpp. */
>+void
>+_dbus_threads_windows_init_global (void)
>+{
>+  InitializeCriticalSection (&init_lock);
>+  global_init_done = TRUE;
>+}
>+
> struct DBusCondVar {
>   DBusList *list;        /**< list thread-local-stored events waiting on the cond variable */
>   CRITICAL_SECTION lock; /**< lock protecting the list */
>@@ -272,3 +283,16 @@ _dbus_threads_init_platform_specific (void)
>   return TRUE;
> }
> 
>+void
>+_dbus_threads_lock_platform_specific (void)
>+{
>+  _dbus_assert (global_init_done);
>+  EnterCriticalSection (&init_lock);
>+}
>+
>+void
>+_dbus_threads_unlock_platform_specific (void)
>+{
>+  _dbus_assert (global_init_done);
>+  LeaveCriticalSection (&init_lock);
>+}
>diff --git a/dbus/dbus-sysdeps-win.h b/dbus/dbus-sysdeps-win.h
>index 74624b7..ae6f6e9 100644
>--- a/dbus/dbus-sysdeps-win.h
>+++ b/dbus/dbus-sysdeps-win.h
>@@ -85,6 +85,8 @@ dbus_bool_t _dbus_get_config_file_name(DBusString *config_file,
> 
> dbus_bool_t _dbus_get_install_root(char *prefix, int len);
> 
>+void        _dbus_threads_windows_init_global (void);
>+
> #endif
> 
> /** @} end of sysdeps-win.h */
>diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
>index a3205cf..d92325c 100644
>--- a/dbus/dbus-sysdeps.h
>+++ b/dbus/dbus-sysdeps.h
>@@ -520,6 +520,18 @@ dbus_bool_t _dbus_read_local_machine_uuid   (DBusGUID         *machine_id,
>  */
> dbus_bool_t _dbus_threads_init_platform_specific (void);
> 
>+/**
>+ * Lock a static mutex used to protect _dbus_threads_init_platform_specific().
>+ *
>+ * On Windows, this is currently unimplemented and does nothing.
>+ */
>+void _dbus_threads_lock_platform_specific (void);
>+
>+/**
>+ * Undo _dbus_threads_lock_platform_specific().
>+ */
>+void _dbus_threads_unlock_platform_specific (void);
>+
> dbus_bool_t _dbus_split_paths_and_append (DBusString *dirs, 
>                                           const char *suffix, 
>                                           DBusList **dir_list);
>diff --git a/dbus/dbus-threads.c b/dbus/dbus-threads.c
>index e7f2eb7..9a505de 100644
>--- a/dbus/dbus-threads.c
>+++ b/dbus/dbus-threads.c
>@@ -581,15 +581,24 @@ init_locks (void)
> dbus_bool_t
> dbus_threads_init (const DBusThreadFunctions *functions)
> {
>+  _dbus_threads_lock_platform_specific ();
>+
>   if (thread_init_generation == _dbus_current_generation)
>-    return TRUE;
>+    {
>+      _dbus_threads_unlock_platform_specific ();
>+      return TRUE;
>+    }
> 
>   if (!_dbus_threads_init_platform_specific() ||
>       !init_locks ())
>-    return FALSE;
>+    {
>+      _dbus_threads_unlock_platform_specific ();
>+      return FALSE;
>+    }
> 
>   thread_init_generation = _dbus_current_generation;
>-  
>+
>+  _dbus_threads_unlock_platform_specific ();
>   return TRUE;
> }
> 
>@@ -600,11 +609,16 @@ dbus_threads_init (const DBusThreadFunctions *functions)
> /**
>  * Initializes threads. If this function is not called, the D-Bus
>  * library will not lock any data structures.  If it is called, D-Bus
>- * will do locking, at some cost in efficiency. Note that this
>- * function must be called BEFORE the second thread is started.
>+ * will do locking, at some cost in efficiency.
>+ *
>+ * Since D-Bus 1.7 it is safe to call this function from any thread,
>+ * any number of times (but it must be called before any other
>+ * libdbus API is used).
>  *
>- * It's safe to call dbus_threads_init_default() as many times as you
>- * want, but only the first time will have an effect.
>+ * In D-Bus 1.6 or older, this function must be called in the main thread
>+ * before any other thread starts. As a result, it is not sufficient to
>+ * call this function in a library or plugin, unless the library or plugin
>+ * imposes a similar requirement on its callers.
>  *
>  * dbus_shutdown() reverses the effects of this function when it
>  * resets all global state in libdbus.
>-- 
>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.