fdo-mirrors/dbus
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/dbus/dbus.git synced 2026-06-04 13:18:18 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

Always initialize threading before allocating a dynamic mutex

Browse source 
Dynamic allocation of mutexes can fail anyway, so this is easy.

Justification for not keeping the dummy mutex code-paths, even as an
opt-in thing for processes known to be high-performance and
single-threaded: real mutexes only cut the throughput of
test/dbus-daemon.c by a couple of percent on my laptop (from around
6700 to around 6600 messages per second), and libdbus crashes caused
by not calling dbus_threads_init_default() are sufficiently widespread
that they're wasting a lot of everyone's time.

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: Anas Nashif <anas.nashif@intel.com>
This commit is contained in:
Simon McVittie 2013-04-16 16:37:51 +01:00
parent 2b3272c75a
commit 08391b1461
1 changed files with 56 additions and 244 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

300
dbus/dbus-threads.c
View file

@ -27,18 +27,6 @@
#include "dbus-list.h"
static int thread_init_generation = 0;
static DBusList *uninitialized_rmutex_list = NULL;
static DBusList *uninitialized_cmutex_list = NULL;
static DBusList *uninitialized_condvar_list = NULL;
/** This is used for the no-op default mutex pointer, just to be distinct from #NULL */
#define _DBUS_DUMMY_MUTEX ((DBusMutex*)0xABCDEF)
#define _DBUS_DUMMY_RMUTEX ((DBusRMutex *) _DBUS_DUMMY_MUTEX)
#define _DBUS_DUMMY_CMUTEX ((DBusCMutex *) _DBUS_DUMMY_MUTEX)
/** This is used for the no-op default mutex pointer, just to be distinct from #NULL */
#define _DBUS_DUMMY_CONDVAR ((DBusCondVar*)0xABCDEF2)
/**
* @defgroup DBusThreadsInternals Thread functions
@ -59,11 +47,6 @@ static DBusList *uninitialized_condvar_list = NULL;
* If possible, the mutex returned by this function is recursive, to
* avoid deadlocks. However, that cannot be relied on.
*
* The extra level of indirection given by allocating a pointer
* to point to the mutex location allows the threading
* module to swap out dummy mutexes for a real mutex so libraries
* can initialize threads even after the D-Bus API has been used.
*
* @param location_p the location of the new mutex, can return #NULL on OOM
*/
void
@ -71,17 +54,13 @@ _dbus_rmutex_new_at_location (DBusRMutex **location_p)
{
_dbus_assert (location_p != NULL);
if (thread_init_generation == _dbus_current_generation)
if (!dbus_threads_init_default ())
{
*location_p = _dbus_platform_rmutex_new ();
*location_p = NULL;
return;
}
else
{
*location_p = _DBUS_DUMMY_RMUTEX;
if (!_dbus_list_append (&uninitialized_rmutex_list, location_p))
*location_p = NULL;
}
*location_p = _dbus_platform_rmutex_new ();
}
/**
@ -92,11 +71,6 @@ _dbus_rmutex_new_at_location (DBusRMutex **location_p)
*
* The returned mutex is suitable for use with condition variables.
*
* The extra level of indirection given by allocating a pointer
* to point to the mutex location allows the threading
* module to swap out dummy mutexes for a real mutex so libraries
* can initialize threads even after the D-Bus API has been used.
*
* @param location_p the location of the new mutex, can return #NULL on OOM
*/
void
@ -104,22 +78,17 @@ _dbus_cmutex_new_at_location (DBusCMutex **location_p)
{
_dbus_assert (location_p != NULL);
if (thread_init_generation == _dbus_current_generation)
if (!dbus_threads_init_default ())
{
*location_p = _dbus_platform_cmutex_new ();
*location_p = NULL;
return;
}
else
{
*location_p = _DBUS_DUMMY_CMUTEX;
if (!_dbus_list_append (&uninitialized_cmutex_list, location_p))
*location_p = NULL;
}
*location_p = _dbus_platform_cmutex_new ();
}
/**
* Frees a DBusRMutex or removes it from the uninitialized mutex list;
* does nothing if passed a #NULL pointer.
* Frees a DBusRMutex; does nothing if passed a #NULL pointer.
*/
void
_dbus_rmutex_free_at_location (DBusRMutex **location_p)
@ -127,23 +96,12 @@ _dbus_rmutex_free_at_location (DBusRMutex **location_p)
if (location_p == NULL)
return;
if (thread_init_generation == _dbus_current_generation)
{
if (*location_p != NULL)
_dbus_platform_rmutex_free (*location_p);
}
else
{
_dbus_assert (*location_p == NULL || *location_p == _DBUS_DUMMY_RMUTEX);
_dbus_list_remove (&uninitialized_rmutex_list, location_p);
}
if (*location_p != NULL)
_dbus_platform_rmutex_free (*location_p);
}
/**
* Frees a DBusCMutex and removes it from the
* uninitialized mutex list;
* does nothing if passed a #NULL pointer.
* Frees a DBusCMutex; does nothing if passed a #NULL pointer.
*/
void
_dbus_cmutex_free_at_location (DBusCMutex **location_p)
@ -151,17 +109,8 @@ _dbus_cmutex_free_at_location (DBusCMutex **location_p)
if (location_p == NULL)
return;
if (thread_init_generation == _dbus_current_generation)
{
if (*location_p != NULL)
_dbus_platform_cmutex_free (*location_p);
}
else
{
_dbus_assert (*location_p == NULL || *location_p == _DBUS_DUMMY_CMUTEX);
_dbus_list_remove (&uninitialized_cmutex_list, location_p);
}
if (*location_p != NULL)
_dbus_platform_cmutex_free (*location_p);
}
/**
@ -172,8 +121,10 @@ _dbus_cmutex_free_at_location (DBusCMutex **location_p)
void
_dbus_rmutex_lock (DBusRMutex *mutex)
{
if (mutex && thread_init_generation == _dbus_current_generation)
_dbus_platform_rmutex_lock (mutex);
if (mutex == NULL)
return;
_dbus_platform_rmutex_lock (mutex);
}
/**
@ -184,8 +135,10 @@ _dbus_rmutex_lock (DBusRMutex *mutex)
void
_dbus_cmutex_lock (DBusCMutex *mutex)
{
if (mutex && thread_init_generation == _dbus_current_generation)
_dbus_platform_cmutex_lock (mutex);
if (mutex == NULL)
return;
_dbus_platform_cmutex_lock (mutex);
}
/**
@ -196,8 +149,10 @@ _dbus_cmutex_lock (DBusCMutex *mutex)
void
_dbus_rmutex_unlock (DBusRMutex *mutex)
{
if (mutex && thread_init_generation == _dbus_current_generation)
_dbus_platform_rmutex_unlock (mutex);
if (mutex == NULL)
return;
_dbus_platform_rmutex_unlock (mutex);
}
/**
@ -208,8 +163,10 @@ _dbus_rmutex_unlock (DBusRMutex *mutex)
void
_dbus_cmutex_unlock (DBusCMutex *mutex)
{
if (mutex && thread_init_generation == _dbus_current_generation)
_dbus_platform_cmutex_unlock (mutex);
if (mutex == NULL)
return;
_dbus_platform_cmutex_unlock (mutex);
}
/**
@ -223,19 +180,17 @@ _dbus_cmutex_unlock (DBusCMutex *mutex)
DBusCondVar *
_dbus_condvar_new (void)
{
if (thread_init_generation == _dbus_current_generation)
return _dbus_platform_condvar_new ();
else
return _DBUS_DUMMY_CONDVAR;
if (!dbus_threads_init_default ())
return NULL;
return _dbus_platform_condvar_new ();
}
/**
* This does the same thing as _dbus_condvar_new. It however
* gives another level of indirection by allocating a pointer
* to point to the condvar location. This allows the threading
* module to swap out dummy condvars for a real condvar so libraries
* can initialize threads even after the D-Bus API has been used.
* to point to the condvar location; this used to be useful.
*
* @returns the location of a new condvar or #NULL on OOM
*/
@ -245,17 +200,7 @@ _dbus_condvar_new_at_location (DBusCondVar **location_p)
{
_dbus_assert (location_p != NULL);
if (thread_init_generation == _dbus_current_generation)
{
*location_p = _dbus_condvar_new();
}
else
{
*location_p = _DBUS_DUMMY_CONDVAR;
if (!_dbus_list_append (&uninitialized_condvar_list, location_p))
*location_p = NULL;
}
*location_p = _dbus_condvar_new();
}
@ -266,14 +211,14 @@ _dbus_condvar_new_at_location (DBusCondVar **location_p)
void
_dbus_condvar_free (DBusCondVar *cond)
{
if (cond && thread_init_generation == _dbus_current_generation)
_dbus_platform_condvar_free (cond);
if (cond == NULL)
return;
_dbus_platform_condvar_free (cond);
}
/**
* Frees a conditional variable and removes it from the
* uninitialized_condvar_list;
* does nothing if passed a #NULL pointer.
* Frees a condition variable; does nothing if passed a #NULL pointer.
*/
void
_dbus_condvar_free_at_location (DBusCondVar **location_p)
@ -281,17 +226,8 @@ _dbus_condvar_free_at_location (DBusCondVar **location_p)
if (location_p == NULL)
return;
if (thread_init_generation == _dbus_current_generation)
{
if (*location_p != NULL)
_dbus_platform_condvar_free (*location_p);
}
else
{
_dbus_assert (*location_p == NULL || *location_p == _DBUS_DUMMY_CONDVAR);
_dbus_list_remove (&uninitialized_condvar_list, location_p);
}
if (*location_p != NULL)
_dbus_platform_condvar_free (*location_p);
}
/**
@ -304,8 +240,10 @@ void
_dbus_condvar_wait (DBusCondVar *cond,
DBusCMutex *mutex)
{
if (cond && mutex && thread_init_generation == _dbus_current_generation)
_dbus_platform_condvar_wait (cond, mutex);
if (cond == NULL || mutex == NULL)
return;
_dbus_platform_condvar_wait (cond, mutex);
}
/**
@ -324,11 +262,11 @@ _dbus_condvar_wait_timeout (DBusCondVar *cond,
DBusCMutex *mutex,
int timeout_milliseconds)
{
if (cond && mutex && thread_init_generation == _dbus_current_generation)
return _dbus_platform_condvar_wait_timeout (cond, mutex,
timeout_milliseconds);
else
if (cond == NULL || mutex == NULL)
return TRUE;
return _dbus_platform_condvar_wait_timeout (cond, mutex,
timeout_milliseconds);
}
/**
@ -339,8 +277,10 @@ _dbus_condvar_wait_timeout (DBusCondVar *cond,
void
_dbus_condvar_wake_one (DBusCondVar *cond)
{
if (cond && thread_init_generation == _dbus_current_generation)
_dbus_platform_condvar_wake_one (cond);
if (cond == NULL)
return;
_dbus_platform_condvar_wake_one (cond);
}
static DBusRMutex *global_locks[_DBUS_N_GLOBAL_LOCKS] = { NULL };
@ -358,132 +298,6 @@ shutdown_global_locks (void *nil)
}
}
static void
shutdown_uninitialized_locks (void *data)
{
_dbus_list_clear (&uninitialized_rmutex_list);
_dbus_list_clear (&uninitialized_cmutex_list);
_dbus_list_clear (&uninitialized_condvar_list);
}
/* init_global_locks() must be called first. */
static dbus_bool_t
init_uninitialized_locks (void)
{
DBusList *link;
dbus_bool_t ok;
_dbus_assert (thread_init_generation != _dbus_current_generation);
link = uninitialized_rmutex_list;
while (link != NULL)
{
DBusRMutex **mp;
mp = link->data;
_dbus_assert (*mp == _DBUS_DUMMY_RMUTEX);
*mp = _dbus_platform_rmutex_new ();
if (*mp == NULL)
goto fail_mutex;
link = _dbus_list_get_next_link (&uninitialized_rmutex_list, link);
}
link = uninitialized_cmutex_list;
while (link != NULL)
{
DBusCMutex **mp;
mp = link->data;
_dbus_assert (*mp == _DBUS_DUMMY_CMUTEX);
*mp = _dbus_platform_cmutex_new ();
if (*mp == NULL)
goto fail_mutex;
link = _dbus_list_get_next_link (&uninitialized_cmutex_list, link);
}
link = uninitialized_condvar_list;
while (link != NULL)
{
DBusCondVar **cp;
cp = (DBusCondVar **)link->data;
_dbus_assert (*cp == _DBUS_DUMMY_CONDVAR);
*cp = _dbus_platform_condvar_new ();
if (*cp == NULL)
goto fail_condvar;
link = _dbus_list_get_next_link (&uninitialized_condvar_list, link);
}
_dbus_list_clear (&uninitialized_rmutex_list);
_dbus_list_clear (&uninitialized_cmutex_list);
_dbus_list_clear (&uninitialized_condvar_list);
/* This assumes that init_global_locks() has already been called. */
_dbus_platform_rmutex_lock (global_locks[_DBUS_LOCK_shutdown_funcs]);
ok = _dbus_register_shutdown_func_unlocked (shutdown_uninitialized_locks, NULL);
_dbus_platform_rmutex_unlock (global_locks[_DBUS_LOCK_shutdown_funcs]);
if (!ok)
goto fail_condvar;
return TRUE;
fail_condvar:
link = uninitialized_condvar_list;
while (link != NULL)
{
DBusCondVar **cp;
cp = link->data;
if (*cp != _DBUS_DUMMY_CONDVAR && *cp != NULL)
_dbus_platform_condvar_free (*cp);
*cp = _DBUS_DUMMY_CONDVAR;
link = _dbus_list_get_next_link (&uninitialized_condvar_list, link);
}
fail_mutex:
link = uninitialized_rmutex_list;
while (link != NULL)
{
DBusRMutex **mp;
mp = link->data;
if (*mp != _DBUS_DUMMY_RMUTEX && *mp != NULL)
_dbus_platform_rmutex_free (*mp);
*mp = _DBUS_DUMMY_RMUTEX;
link = _dbus_list_get_next_link (&uninitialized_rmutex_list, link);
}
link = uninitialized_cmutex_list;
while (link != NULL)
{
DBusCMutex **mp;
mp = link->data;
if (*mp != _DBUS_DUMMY_CMUTEX && *mp != NULL)
_dbus_platform_cmutex_free (*mp);
*mp = _DBUS_DUMMY_CMUTEX;
link = _dbus_list_get_next_link (&uninitialized_cmutex_list, link);
}
return FALSE;
}
static dbus_bool_t
init_global_locks (void)
{
@ -585,9 +399,7 @@ dbus_threads_init (const DBusThreadFunctions *functions)
}
if (!_dbus_threads_init_platform_specific() ||
/* init_global_locks() must be called before init_uninitialized_locks. */
!init_global_locks () ||
!init_uninitialized_locks ())
!init_global_locks ())
{
_dbus_threads_unlock_platform_specific ();
return FALSE;