fdo-mirrors/dbus
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/dbus/dbus.git synced 2026-05-02 09:38:23 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

* dbus/dbus-threads.c: Allow recursive mutex's to be passed into

Browse source 
dbus_threads_init and be used by the dbus mutex functions to
  avoid deadlocks.

* doc/TODO: Remove recursive mutex dbus_connection_dispatch TODO item
This commit is contained in:
John (J5) Palmieri 2006-09-14 04:26:00 +00:00
parent f82bdd3ab3
commit 57ab23491c
7 changed files with 109 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
ChangeLog
View file

@ -1,3 +1,11 @@
2006-09-14 John (J5) Palmieri <johnp@redhat.com>
* dbus/dbus-threads.c: Allow recursive mutex's to be passed into
dbus_threads_init and be used by the dbus mutex functions to
avoid deadlocks.
* doc/TODO: Remove recursive mutex dbus_connection_dispatch TODO item
2006-09-13 John (J5) Palmieri <johnp@redhat.com>
* dbus/dbus-sysdeps-util-unix.c (_dbus_directory_get_next_file):

9
dbus/dbus-dataslot.c
View file

@ -70,8 +70,7 @@ _dbus_data_slot_allocator_alloc (DBusDataSlotAllocator *allocator,
{
dbus_int32_t slot;
if (!_dbus_mutex_lock (*mutex_loc))
return FALSE;
_dbus_mutex_lock (*mutex_loc);
if (allocator->n_allocated_slots == 0)
{
@ -246,8 +245,7 @@ _dbus_data_slot_list_set (DBusDataSlotAllocator *allocator,
* be e.g. realloc()ing allocated_slots. We avoid doing this if asserts
* are disabled, since then the asserts are empty.
*/
if (!_dbus_mutex_lock (*(allocator->lock_loc)))
return FALSE;
_dbus_mutex_lock (*(allocator->lock_loc));
_dbus_assert (slot < allocator->n_allocated_slots);
_dbus_assert (allocator->allocated_slots[slot].slot_id == slot);
_dbus_mutex_unlock (*(allocator->lock_loc));
@ -304,8 +302,7 @@ _dbus_data_slot_list_get (DBusDataSlotAllocator *allocator,
* be e.g. realloc()ing allocated_slots. We avoid doing this if asserts
* are disabled, since then the asserts are empty.
*/
if (!_dbus_mutex_lock (*(allocator->lock_loc)))
return NULL;
_dbus_mutex_lock (*(allocator->lock_loc));
_dbus_assert (slot >= 0);
_dbus_assert (slot < allocator->n_allocated_slots);
_dbus_assert (allocator->allocated_slots[slot].slot_id == slot);

3
dbus/dbus-list.c
View file

@ -55,8 +55,7 @@ alloc_link (void *data)
{
DBusList *link;
if (!_DBUS_LOCK (list))
return NULL;
_DBUS_LOCK (list);
if (list_pool == NULL)
{

4
dbus/dbus-threads-internal.h
View file

@ -31,8 +31,8 @@ DBUS_BEGIN_DECLS
DBusMutex* _dbus_mutex_new (void);
void _dbus_mutex_free (DBusMutex *mutex);
dbus_bool_t _dbus_mutex_lock (DBusMutex *mutex);
dbus_bool_t _dbus_mutex_unlock (DBusMutex *mutex);
void _dbus_mutex_lock (DBusMutex *mutex);
void _dbus_mutex_unlock (DBusMutex *mutex);
void _dbus_mutex_new_at_location (DBusMutex **location_p);
void _dbus_mutex_free_at_location (DBusMutex **location_p);

105
dbus/dbus-threads.c
View file

@ -39,10 +39,10 @@
static DBusThreadFunctions thread_functions =
{
0,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL
};
@ -87,7 +87,9 @@ static DBusList *uninitialized_condvar_list = NULL;
DBusMutex*
_dbus_mutex_new (void)
{
if (thread_functions.mutex_new)
if (thread_functions.recursive_mutex_new)
return (* thread_functions.recursive_mutex_new) ();
else if (thread_functions.mutex_new)
return (* thread_functions.mutex_new) ();
else
return _DBUS_DUMMY_MUTEX;
@ -126,8 +128,13 @@ _dbus_mutex_new_at_location (DBusMutex **location_p)
void
_dbus_mutex_free (DBusMutex *mutex)
{
if (mutex && thread_functions.mutex_free)
(* thread_functions.mutex_free) (mutex);
if (mutex)
{
if (mutex && thread_functions.recursive_mutex_free)
(* thread_functions.recursive_mutex_free) (mutex);
else if (mutex && thread_functions.mutex_free)
(* thread_functions.mutex_free) (mutex);
}
}
/**
@ -149,17 +156,19 @@ _dbus_mutex_free_at_location (DBusMutex **location_p)
/**
* Locks a mutex. Does nothing if passed a #NULL pointer.
* Locks are not recursive.
*
* @returns #TRUE on success
* Locks may be recursive if threading implementation initialized
* recursive locks.
*/
dbus_bool_t
void
_dbus_mutex_lock (DBusMutex *mutex)
{
if (mutex && thread_functions.mutex_lock)
return (* thread_functions.mutex_lock) (mutex);
else
return TRUE;
if (mutex)
{
if (thread_functions.recursive_mutex_lock)
(* thread_functions.recursive_mutex_lock) (mutex);
else if (thread_functions.mutex_lock)
(* thread_functions.mutex_lock) (mutex);
}
}
/**
@ -167,13 +176,16 @@ _dbus_mutex_lock (DBusMutex *mutex)
*
* @returns #TRUE on success
*/
dbus_bool_t
void
_dbus_mutex_unlock (DBusMutex *mutex)
{
if (mutex && thread_functions.mutex_unlock)
return (* thread_functions.mutex_unlock) (mutex);
else
return TRUE;
if (mutex)
{
if (thread_functions.recursive_mutex_unlock)
(* thread_functions.recursive_mutex_unlock) (mutex);
else if (thread_functions.mutex_unlock)
(* thread_functions.mutex_unlock) (mutex);
}
}
/**
@ -515,25 +527,20 @@ init_locks (void)
dbus_bool_t
dbus_threads_init (const DBusThreadFunctions *functions)
{
dbus_bool_t mutex_set;
dbus_bool_t recursive_mutex_set;
_dbus_assert (functions != NULL);
/* these base functions are required. Future additions to
* DBusThreadFunctions may be optional.
*/
_dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_NEW_MASK);
_dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_FREE_MASK);
_dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_LOCK_MASK);
_dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_UNLOCK_MASK);
_dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_NEW_MASK);
_dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_FREE_MASK);
_dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_MASK);
_dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_TIMEOUT_MASK);
_dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ONE_MASK);
_dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ALL_MASK);
_dbus_assert (functions->mutex_new != NULL);
_dbus_assert (functions->mutex_free != NULL);
_dbus_assert (functions->mutex_lock != NULL);
_dbus_assert (functions->mutex_unlock != NULL);
_dbus_assert (functions->condvar_new != NULL);
_dbus_assert (functions->condvar_free != NULL);
_dbus_assert (functions->condvar_wait != NULL);
@ -541,6 +548,40 @@ dbus_threads_init (const DBusThreadFunctions *functions)
_dbus_assert (functions->condvar_wake_one != NULL);
_dbus_assert (functions->condvar_wake_all != NULL);
/* Either the mutex function set or recursive mutex set needs
* to be available but not both
*/
mutex_set = (functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_NEW_MASK) &&
(functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_FREE_MASK) &&
(functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_LOCK_MASK) &&
(functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_UNLOCK_MASK) &&
functions->mutex_new &&
functions->mutex_free &&
functions->mutex_lock &&
functions->mutex_unlock;
recursive_mutex_set =
(functions->mask & DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_NEW_MASK) &&
(functions->mask & DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_FREE_MASK) &&
(functions->mask & DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_LOCK_MASK) &&
(functions->mask & DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_UNLOCK_MASK) &&
functions->recursive_mutex_new &&
functions->recursive_mutex_free &&
functions->recursive_mutex_lock &&
functions->recursive_mutex_unlock;
if (!(mutex_set || recursive_mutex_set))
_dbus_assert_not_reached ("Either the nonrecusrive or recursive mutex "
"functions sets should be passed into "
"dbus_threads_init. Neither sets were passed.");
if (mutex_set && recursive_mutex_set)
_dbus_assert_not_reached ("Either the nonrecusrive or recursive mutex "
"functions sets should be passed into "
"dbus_threads_init. Both sets were passed. "
"You most likely just want to set the recursive "
"mutex functions to avoid deadlocks in D-Bus.");
/* Check that all bits in the mask actually are valid mask bits.
* ensures people won't write code that breaks when we add
* new bits.
@ -567,7 +608,19 @@ dbus_threads_init (const DBusThreadFunctions *functions)
thread_functions.condvar_wait_timeout = functions->condvar_wait_timeout;
thread_functions.condvar_wake_one = functions->condvar_wake_one;
thread_functions.condvar_wake_all = functions->condvar_wake_all;
if (functions->mask & DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_NEW_MASK)
thread_functions.recursive_mutex_new = functions->recursive_mutex_new;
if (functions->mask & DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_FREE_MASK)
thread_functions.recursive_mutex_free = functions->recursive_mutex_free;
if (functions->mask & DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_LOCK_MASK)
thread_functions.recursive_mutex_lock = functions->recursive_mutex_lock;
if (functions->mask & DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_UNLOCK_MASK)
thread_functions.recursive_mutex_unlock = functions->recursive_mutex_unlock;
thread_functions.mask = functions->mask;
if (!init_locks ())

23
dbus/dbus-threads.h
View file

@ -40,6 +40,11 @@ typedef void (* DBusMutexFreeFunction) (DBusMutex *mutex);
typedef dbus_bool_t (* DBusMutexLockFunction) (DBusMutex *mutex);
typedef dbus_bool_t (* DBusMutexUnlockFunction) (DBusMutex *mutex);
typedef DBusMutex* (* DBusRecursiveMutexNewFunction) (void);
typedef void (* DBusRecursiveMutexFreeFunction) (DBusMutex *mutex);
typedef void (* DBusRecursiveMutexLockFunction) (DBusMutex *mutex);
typedef void (* DBusRecursiveMutexUnlockFunction) (DBusMutex *mutex);
typedef DBusCondVar* (* DBusCondVarNewFunction) (void);
typedef void (* DBusCondVarFreeFunction) (DBusCondVar *cond);
typedef void (* DBusCondVarWaitFunction) (DBusCondVar *cond,
@ -62,8 +67,11 @@ typedef enum
DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_TIMEOUT_MASK = 1 << 7,
DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ONE_MASK = 1 << 8,
DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ALL_MASK = 1 << 9,
DBUS_THREAD_FUNCTIONS_ALL_MASK = (1 << 10) - 1
DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_NEW_MASK = 1 << 10,
DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_FREE_MASK = 1 << 11,
DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_LOCK_MASK = 1 << 12,
DBUS_THREAD_FUNCTIONS_RECURSIVE_MUTEX_UNLOCK_MASK = 1 << 13,
DBUS_THREAD_FUNCTIONS_ALL_MASK = (1 << 13) - 1
} DBusThreadFunctionsMask;
/**
@ -85,15 +93,16 @@ typedef struct
DBusCondVarWaitTimeoutFunction condvar_wait_timeout; /**< Function to wait on a condition with a timeout */
DBusCondVarWakeOneFunction condvar_wake_one; /**< Function to wake one thread waiting on the condition */
DBusCondVarWakeAllFunction condvar_wake_all; /**< Function to wake all threads waiting on the condition */
DBusRecursiveMutexNewFunction recursive_mutex_new; /**< Function to create a recursive mutex */
DBusRecursiveMutexFreeFunction recursive_mutex_free; /**< Function to free a recursive mutex */
DBusRecursiveMutexLockFunction recursive_mutex_lock; /**< Function to lock a recursive mutex */
DBusRecursiveMutexUnlockFunction recursive_mutex_unlock; /**< Function to unlock a recursive mutex */
void (* padding1) (void); /**< Reserved for future expansion */
void (* padding2) (void); /**< Reserved for future expansion */
void (* padding3) (void); /**< Reserved for future expansion */
void (* padding4) (void); /**< Reserved for future expansion */
void (* padding5) (void); /**< Reserved for future expansion */
void (* padding6) (void); /**< Reserved for future expansion */
void (* padding7) (void); /**< Reserved for future expansion */
void (* padding8) (void); /**< Reserved for future expansion */
} DBusThreadFunctions;

7
doc/TODO
View file

@ -1,13 +1,6 @@
Important for 1.0
===
- add a new return code from dbus_connection_dispatch() called
IN_PROGRESS or RECURSED or something, indicating that DATA_REMAINS
but another dispatch is in progress, so we can't dispatch at
this time. OR maybe just switch to recursive locks for the dispatch
locks. Fixes the recursive deadlock. See the @todo for more
and this thread: http://lists.freedesktop.org/archives/dbus/2006-February/004128.html
- Take a look at the issues marked @todo 1.0 or FIXME 1.0. Ones with
Question marks at the ends either need clarification or are not
really needed for 1.0 but would be nice.