From baf1a8991f31239d3f9d37d2a14b7c5764145bd8 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 14 Feb 2012 19:58:56 +0000 Subject: [PATCH 2/4] Distinguish between two flavours of mutex dbus-threads.h warns that recursive pthreads mutexes are not compatible with our expectations for condition variables. However, the only two condition variables we actually use only have their corresponding mutexes locked briefly (and we don't call out to user code from there), so the mutexes don't need to be recursive anyway. That's just as well, because it turns out our implementation of recursive mutexes on pthreads is broken! The goal here is to be able to distinguish between "cmutexes" (mutexes compatible with a condition variable) and "rmutexes" (mutexes which are recursive if possible, to avoid deadlocking if we hold them while calling user code). This is complicated by the fact that callers are not guaranteed to have provided us with both versions of mutexes, so we might have to implement one by using the other (in particular, DBusRMutex *aims to be* recursive, it is not *guaranteed to be* recursive). Bug: https://bugs.freedesktop.org/show_bug.cgi?id=43744 Signed-off-by: Simon McVittie --- dbus/dbus-connection.c | 62 ++++++------ dbus/dbus-dataslot.c | 30 +++--- dbus/dbus-dataslot.h | 4 +- dbus/dbus-internals.h | 8 +- dbus/dbus-server-protected.h | 6 +- dbus/dbus-server.c | 8 +- dbus/dbus-threads-internal.h | 36 +++++- dbus/dbus-threads.c | 243 +++++++++++++++++++++++++++++++++--------- 8 files changed, 283 insertions(+), 114 deletions(-) diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index b8f48ad..74a9007 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -66,18 +66,18 @@ #define CONNECTION_LOCK(connection) do { \ if (TRACE_LOCKS) { _dbus_verbose ("LOCK\n"); } \ - _dbus_mutex_lock ((connection)->mutex); \ + _dbus_rmutex_lock ((connection)->mutex); \ TOOK_LOCK_CHECK (connection); \ } while (0) #define CONNECTION_UNLOCK(connection) _dbus_connection_unlock (connection) #define SLOTS_LOCK(connection) do { \ - _dbus_mutex_lock ((connection)->slot_mutex); \ + _dbus_rmutex_lock ((connection)->slot_mutex); \ } while (0) #define SLOTS_UNLOCK(connection) do { \ - _dbus_mutex_unlock ((connection)->slot_mutex); \ + _dbus_rmutex_unlock ((connection)->slot_mutex); \ } while (0) #define DISPATCH_STATUS_NAME(s) \ @@ -264,11 +264,11 @@ struct DBusConnection { DBusAtomic refcount; /**< Reference count. */ - DBusMutex *mutex; /**< Lock on the entire DBusConnection */ + DBusRMutex *mutex; /**< Lock on the entire DBusConnection */ - DBusMutex *dispatch_mutex; /**< Protects dispatch_acquired */ + DBusCMutex *dispatch_mutex; /**< Protects dispatch_acquired */ DBusCondVar *dispatch_cond; /**< Notify when dispatch_acquired is available */ - DBusMutex *io_path_mutex; /**< Protects io_path_acquired */ + DBusCMutex *io_path_mutex; /**< Protects io_path_acquired */ DBusCondVar *io_path_cond; /**< Notify when io_path_acquired is available */ DBusList *outgoing_messages; /**< Queue of messages we need to send, send the end of the list first. */ @@ -290,7 +290,7 @@ struct DBusConnection DBusList *filter_list; /**< List of filters. */ - DBusMutex *slot_mutex; /**< Lock on slot_list so overall connection lock need not be taken */ + DBusRMutex *slot_mutex; /**< Lock on slot_list so overall connection lock need not be taken */ DBusDataSlotList slot_list; /**< Data stored by allocated integer ID */ DBusHashTable *pending_replies; /**< Hash of message serials to #DBusPendingCall. */ @@ -419,7 +419,7 @@ _dbus_connection_unlock (DBusConnection *connection) connection->expired_messages = NULL; RELEASING_LOCK_CHECK (connection); - _dbus_mutex_unlock (connection->mutex); + _dbus_rmutex_unlock (connection->mutex); for (iter = _dbus_list_pop_first_link (&expired_messages); iter != NULL; @@ -467,9 +467,9 @@ _dbus_connection_test_get_locks (DBusConnection *connection, DBusCondVar **dispatch_cond_loc, DBusCondVar **io_path_cond_loc) { - *mutex_loc = connection->mutex; - *dispatch_mutex_loc = connection->dispatch_mutex; - *io_path_mutex_loc = connection->io_path_mutex; + *mutex_loc = (DBusMutex *) connection->mutex; + *dispatch_mutex_loc = (DBusMutex *) connection->dispatch_mutex; + *io_path_mutex_loc = (DBusMutex *) connection->io_path_mutex; *dispatch_cond_loc = connection->dispatch_cond; *io_path_cond_loc = connection->io_path_cond; } @@ -1079,7 +1079,7 @@ _dbus_connection_acquire_io_path (DBusConnection *connection, CONNECTION_UNLOCK (connection); _dbus_verbose ("locking io_path_mutex\n"); - _dbus_mutex_lock (connection->io_path_mutex); + _dbus_cmutex_lock (connection->io_path_mutex); _dbus_verbose ("start connection->io_path_acquired = %d timeout = %d\n", connection->io_path_acquired, timeout_milliseconds); @@ -1128,7 +1128,7 @@ _dbus_connection_acquire_io_path (DBusConnection *connection, connection->io_path_acquired, we_acquired); _dbus_verbose ("unlocking io_path_mutex\n"); - _dbus_mutex_unlock (connection->io_path_mutex); + _dbus_cmutex_unlock (connection->io_path_mutex); CONNECTION_LOCK (connection); @@ -1152,7 +1152,7 @@ _dbus_connection_release_io_path (DBusConnection *connection) HAVE_LOCK_CHECK (connection); _dbus_verbose ("locking io_path_mutex\n"); - _dbus_mutex_lock (connection->io_path_mutex); + _dbus_cmutex_lock (connection->io_path_mutex); _dbus_assert (connection->io_path_acquired); @@ -1163,7 +1163,7 @@ _dbus_connection_release_io_path (DBusConnection *connection) _dbus_condvar_wake_one (connection->io_path_cond); _dbus_verbose ("unlocking io_path_mutex\n"); - _dbus_mutex_unlock (connection->io_path_mutex); + _dbus_cmutex_unlock (connection->io_path_mutex); } /** @@ -1292,15 +1292,15 @@ _dbus_connection_new_for_transport (DBusTransport *transport) if (connection == NULL) goto error; - _dbus_mutex_new_at_location (&connection->mutex); + _dbus_rmutex_new_at_location (&connection->mutex); if (connection->mutex == NULL) goto error; - _dbus_mutex_new_at_location (&connection->io_path_mutex); + _dbus_cmutex_new_at_location (&connection->io_path_mutex); if (connection->io_path_mutex == NULL) goto error; - _dbus_mutex_new_at_location (&connection->dispatch_mutex); + _dbus_cmutex_new_at_location (&connection->dispatch_mutex); if (connection->dispatch_mutex == NULL) goto error; @@ -1312,7 +1312,7 @@ _dbus_connection_new_for_transport (DBusTransport *transport) if (connection->io_path_cond == NULL) goto error; - _dbus_mutex_new_at_location (&connection->slot_mutex); + _dbus_rmutex_new_at_location (&connection->slot_mutex); if (connection->slot_mutex == NULL) goto error; @@ -1391,10 +1391,10 @@ _dbus_connection_new_for_transport (DBusTransport *transport) { _dbus_condvar_free_at_location (&connection->io_path_cond); _dbus_condvar_free_at_location (&connection->dispatch_cond); - _dbus_mutex_free_at_location (&connection->mutex); - _dbus_mutex_free_at_location (&connection->io_path_mutex); - _dbus_mutex_free_at_location (&connection->dispatch_mutex); - _dbus_mutex_free_at_location (&connection->slot_mutex); + _dbus_rmutex_free_at_location (&connection->mutex); + _dbus_cmutex_free_at_location (&connection->io_path_mutex); + _dbus_cmutex_free_at_location (&connection->dispatch_mutex); + _dbus_rmutex_free_at_location (&connection->slot_mutex); dbus_free (connection); } if (pending_replies) @@ -2739,12 +2739,12 @@ _dbus_connection_last_unref (DBusConnection *connection) _dbus_condvar_free_at_location (&connection->dispatch_cond); _dbus_condvar_free_at_location (&connection->io_path_cond); - _dbus_mutex_free_at_location (&connection->io_path_mutex); - _dbus_mutex_free_at_location (&connection->dispatch_mutex); + _dbus_cmutex_free_at_location (&connection->io_path_mutex); + _dbus_cmutex_free_at_location (&connection->dispatch_mutex); - _dbus_mutex_free_at_location (&connection->slot_mutex); + _dbus_rmutex_free_at_location (&connection->slot_mutex); - _dbus_mutex_free_at_location (&connection->mutex); + _dbus_rmutex_free_at_location (&connection->mutex); dbus_free (connection); } @@ -4088,7 +4088,7 @@ _dbus_connection_acquire_dispatch (DBusConnection *connection) CONNECTION_UNLOCK (connection); _dbus_verbose ("locking dispatch_mutex\n"); - _dbus_mutex_lock (connection->dispatch_mutex); + _dbus_cmutex_lock (connection->dispatch_mutex); while (connection->dispatch_acquired) { @@ -4102,7 +4102,7 @@ _dbus_connection_acquire_dispatch (DBusConnection *connection) connection->dispatch_acquired = TRUE; _dbus_verbose ("unlocking dispatch_mutex\n"); - _dbus_mutex_unlock (connection->dispatch_mutex); + _dbus_cmutex_unlock (connection->dispatch_mutex); CONNECTION_LOCK (connection); _dbus_connection_unref_unlocked (connection); @@ -4121,7 +4121,7 @@ _dbus_connection_release_dispatch (DBusConnection *connection) HAVE_LOCK_CHECK (connection); _dbus_verbose ("locking dispatch_mutex\n"); - _dbus_mutex_lock (connection->dispatch_mutex); + _dbus_cmutex_lock (connection->dispatch_mutex); _dbus_assert (connection->dispatch_acquired); @@ -4129,7 +4129,7 @@ _dbus_connection_release_dispatch (DBusConnection *connection) _dbus_condvar_wake_one (connection->dispatch_cond); _dbus_verbose ("unlocking dispatch_mutex\n"); - _dbus_mutex_unlock (connection->dispatch_mutex); + _dbus_cmutex_unlock (connection->dispatch_mutex); } static void diff --git a/dbus/dbus-dataslot.c b/dbus/dbus-dataslot.c index fd25c41..0369612 100644 --- a/dbus/dbus-dataslot.c +++ b/dbus/dbus-dataslot.c @@ -67,12 +67,12 @@ _dbus_data_slot_allocator_init (DBusDataSlotAllocator *allocator) */ dbus_bool_t _dbus_data_slot_allocator_alloc (DBusDataSlotAllocator *allocator, - DBusMutex **mutex_loc, + DBusRMutex **mutex_loc, dbus_int32_t *slot_id_p) { dbus_int32_t slot; - _dbus_mutex_lock (*mutex_loc); + _dbus_rmutex_lock (*mutex_loc); if (allocator->n_allocated_slots == 0) { @@ -146,7 +146,7 @@ _dbus_data_slot_allocator_alloc (DBusDataSlotAllocator *allocator, slot, allocator, allocator->n_allocated_slots, allocator->n_used_slots); out: - _dbus_mutex_unlock (*(allocator->lock_loc)); + _dbus_rmutex_unlock (*(allocator->lock_loc)); return slot >= 0; } @@ -165,7 +165,7 @@ void _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator, dbus_int32_t *slot_id_p) { - _dbus_mutex_lock (*(allocator->lock_loc)); + _dbus_rmutex_lock (*(allocator->lock_loc)); _dbus_assert (*slot_id_p < allocator->n_allocated_slots); _dbus_assert (allocator->allocated_slots[*slot_id_p].slot_id == *slot_id_p); @@ -175,7 +175,7 @@ _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator, if (allocator->allocated_slots[*slot_id_p].refcount > 0) { - _dbus_mutex_unlock (*(allocator->lock_loc)); + _dbus_rmutex_unlock (*(allocator->lock_loc)); return; } @@ -190,18 +190,18 @@ _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator, if (allocator->n_used_slots == 0) { - DBusMutex **mutex_loc = allocator->lock_loc; + DBusRMutex **mutex_loc = allocator->lock_loc; dbus_free (allocator->allocated_slots); allocator->allocated_slots = NULL; allocator->n_allocated_slots = 0; allocator->lock_loc = NULL; - _dbus_mutex_unlock (*mutex_loc); + _dbus_rmutex_unlock (*mutex_loc); } else { - _dbus_mutex_unlock (*(allocator->lock_loc)); + _dbus_rmutex_unlock (*(allocator->lock_loc)); } } @@ -247,10 +247,10 @@ _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. */ - _dbus_mutex_lock (*(allocator->lock_loc)); + _dbus_rmutex_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)); + _dbus_rmutex_unlock (*(allocator->lock_loc)); #endif if (slot >= list->n_slots) @@ -304,11 +304,11 @@ _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. */ - _dbus_mutex_lock (*(allocator->lock_loc)); + _dbus_rmutex_lock (*(allocator->lock_loc)); _dbus_assert (slot >= 0); _dbus_assert (slot < allocator->n_allocated_slots); _dbus_assert (allocator->allocated_slots[slot].slot_id == slot); - _dbus_mutex_unlock (*(allocator->lock_loc)); + _dbus_rmutex_unlock (*(allocator->lock_loc)); #endif if (slot >= list->n_slots) @@ -384,14 +384,14 @@ _dbus_data_slot_test (void) int i; DBusFreeFunction old_free_func; void *old_data; - DBusMutex *mutex; + DBusRMutex *mutex; if (!_dbus_data_slot_allocator_init (&allocator)) _dbus_assert_not_reached ("no memory for allocator"); _dbus_data_slot_list_init (&list); - _dbus_mutex_new_at_location (&mutex); + _dbus_rmutex_new_at_location (&mutex); if (mutex == NULL) _dbus_assert_not_reached ("failed to alloc mutex"); @@ -471,7 +471,7 @@ _dbus_data_slot_test (void) ++i; } - _dbus_mutex_free_at_location (&mutex); + _dbus_rmutex_free_at_location (&mutex); return TRUE; } diff --git a/dbus/dbus-dataslot.h b/dbus/dbus-dataslot.h index 2e706f7..3d9d5ed 100644 --- a/dbus/dbus-dataslot.h +++ b/dbus/dbus-dataslot.h @@ -57,7 +57,7 @@ struct DBusDataSlotAllocator DBusAllocatedSlot *allocated_slots; /**< Allocated slots */ int n_allocated_slots; /**< number of slots malloc'd */ int n_used_slots; /**< number of slots used */ - DBusMutex **lock_loc; /**< location of thread lock */ + DBusRMutex **lock_loc; /**< location of thread lock */ }; /** @@ -72,7 +72,7 @@ struct DBusDataSlotList dbus_bool_t _dbus_data_slot_allocator_init (DBusDataSlotAllocator *allocator); dbus_bool_t _dbus_data_slot_allocator_alloc (DBusDataSlotAllocator *allocator, - DBusMutex **mutex_loc, + DBusRMutex **mutex_loc, int *slot_id_p); void _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator, int *slot_id_p); diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index 162a222..8036a2b 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -304,10 +304,10 @@ extern int _dbus_current_generation; /* Thread initializers */ #define _DBUS_LOCK_NAME(name) _dbus_lock_##name -#define _DBUS_DECLARE_GLOBAL_LOCK(name) extern DBusMutex *_dbus_lock_##name -#define _DBUS_DEFINE_GLOBAL_LOCK(name) DBusMutex *_dbus_lock_##name -#define _DBUS_LOCK(name) _dbus_mutex_lock (_dbus_lock_##name) -#define _DBUS_UNLOCK(name) _dbus_mutex_unlock (_dbus_lock_##name) +#define _DBUS_DECLARE_GLOBAL_LOCK(name) extern DBusRMutex *_dbus_lock_##name +#define _DBUS_DEFINE_GLOBAL_LOCK(name) DBusRMutex *_dbus_lock_##name +#define _DBUS_LOCK(name) _dbus_rmutex_lock (_dbus_lock_##name) +#define _DBUS_UNLOCK(name) _dbus_rmutex_unlock (_dbus_lock_##name) /* 1-5 */ _DBUS_DECLARE_GLOBAL_LOCK (list); diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h index df306f7..dd5234b 100644 --- a/dbus/dbus-server-protected.h +++ b/dbus/dbus-server-protected.h @@ -57,7 +57,7 @@ struct DBusServer { DBusAtomic refcount; /**< Reference count. */ const DBusServerVTable *vtable; /**< Virtual methods for this instance. */ - DBusMutex *mutex; /**< Lock on the server object */ + DBusRMutex *mutex; /**< Lock on the server object */ DBusGUID guid; /**< Globally unique ID of server */ @@ -161,14 +161,14 @@ void _dbus_server_trace_ref (DBusServer *server, #define SERVER_LOCK(server) do { \ if (TRACE_LOCKS) { _dbus_verbose ("LOCK\n"); } \ - _dbus_mutex_lock ((server)->mutex); \ + _dbus_rmutex_lock ((server)->mutex); \ TOOK_LOCK_CHECK (server); \ } while (0) #define SERVER_UNLOCK(server) do { \ if (TRACE_LOCKS) { _dbus_verbose ("UNLOCK\n"); } \ RELEASING_LOCK_CHECK (server); \ - _dbus_mutex_unlock ((server)->mutex); \ + _dbus_rmutex_unlock ((server)->mutex); \ } while (0) DBUS_END_DECLS diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c index 157557c..b62c2b4 100644 --- a/dbus/dbus-server.c +++ b/dbus/dbus-server.c @@ -142,7 +142,7 @@ _dbus_server_init_base (DBusServer *server, if (server->address == NULL) goto failed; - _dbus_mutex_new_at_location (&server->mutex); + _dbus_rmutex_new_at_location (&server->mutex); if (server->mutex == NULL) goto failed; @@ -161,7 +161,7 @@ _dbus_server_init_base (DBusServer *server, return TRUE; failed: - _dbus_mutex_free_at_location (&server->mutex); + _dbus_rmutex_free_at_location (&server->mutex); server->mutex = NULL; if (server->watches) { @@ -208,7 +208,7 @@ _dbus_server_finalize_base (DBusServer *server) _dbus_watch_list_free (server->watches); _dbus_timeout_list_free (server->timeouts); - _dbus_mutex_free_at_location (&server->mutex); + _dbus_rmutex_free_at_location (&server->mutex); dbus_free (server->address); @@ -1093,7 +1093,7 @@ dbus_bool_t dbus_server_allocate_data_slot (dbus_int32_t *slot_p) { return _dbus_data_slot_allocator_alloc (&slot_allocator, - (DBusMutex **)&_DBUS_LOCK_NAME (server_slots), + (DBusRMutex **)&_DBUS_LOCK_NAME (server_slots), slot_p); } diff --git a/dbus/dbus-threads-internal.h b/dbus/dbus-threads-internal.h index 68aa20a..2561136 100644 --- a/dbus/dbus-threads-internal.h +++ b/dbus/dbus-threads-internal.h @@ -27,19 +27,43 @@ #include #include +/** + * @addtogroup DBusThreadsInternals + * @{ + */ + +/** + * A mutex which is recursive if possible, else non-recursive. + * This is typically recursive, but that cannot be relied upon. + */ +typedef struct DBusRMutex DBusRMutex; + +/** + * A mutex suitable for use with condition variables. + * This is typically non-recursive. + */ +typedef struct DBusCMutex DBusCMutex; + +/** @} */ + DBUS_BEGIN_DECLS -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); +void _dbus_rmutex_lock (DBusRMutex *mutex); +void _dbus_rmutex_unlock (DBusRMutex *mutex); +void _dbus_rmutex_new_at_location (DBusRMutex **location_p); +void _dbus_rmutex_free_at_location (DBusRMutex **location_p); + +void _dbus_cmutex_lock (DBusCMutex *mutex); +void _dbus_cmutex_unlock (DBusCMutex *mutex); +void _dbus_cmutex_new_at_location (DBusCMutex **location_p); +void _dbus_cmutex_free_at_location (DBusCMutex **location_p); DBusCondVar* _dbus_condvar_new (void); void _dbus_condvar_free (DBusCondVar *cond); void _dbus_condvar_wait (DBusCondVar *cond, - DBusMutex *mutex); + DBusCMutex *mutex); dbus_bool_t _dbus_condvar_wait_timeout (DBusCondVar *cond, - DBusMutex *mutex, + DBusCMutex *mutex, int timeout_milliseconds); void _dbus_condvar_wake_one (DBusCondVar *cond); void _dbus_condvar_wake_all (DBusCondVar *cond); diff --git a/dbus/dbus-threads.c b/dbus/dbus-threads.c index 5eda6f1..eff3054 100644 --- a/dbus/dbus-threads.c +++ b/dbus/dbus-threads.c @@ -38,7 +38,8 @@ static DBusThreadFunctions thread_functions = static int thread_init_generation = 0; -static DBusList *uninitialized_mutex_list = NULL; +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 */ @@ -47,12 +48,19 @@ 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_CONDVAR ((DBusCondVar*)0xABCDEF2) -static void _dbus_mutex_free (DBusMutex *mutex); +static void _dbus_mutex_free (DBusMutex *mutex, DBusMutexFreeFunction dtor); +static void _dbus_mutex_new_at_location (DBusMutex **location_p, + DBusMutexNewFunction ctor, + DBusMutexFreeFunction dtor, + DBusList **uninitialized); +static void _dbus_mutex_free_at_location (DBusMutex **location_p, + DBusMutexFreeFunction dtor, + DBusList **uninitialized); /** * @defgroup DBusThreadsInternals Thread functions * @ingroup DBusInternals - * @brief _dbus_mutex_lock(), etc. + * @brief _dbus_rmutex_lock(), etc. * * Functions and macros related to threads and thread locks. * @@ -60,9 +68,11 @@ static void _dbus_mutex_free (DBusMutex *mutex); */ static DBusMutex * -_dbus_mutex_new (void) +_dbus_mutex_new (DBusMutexNewFunction ctor) { - if (thread_functions.recursive_mutex_new) + if (ctor) + return ctor (); + else if (thread_functions.recursive_mutex_new) return (* thread_functions.recursive_mutex_new) (); else if (thread_functions.mutex_new) return (* thread_functions.mutex_new) (); @@ -76,6 +86,33 @@ _dbus_mutex_new (void) * May return #NULL even if threads are initialized, indicating * out-of-memory. * + * 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 real 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 +_dbus_rmutex_new_at_location (DBusRMutex **location_p) +{ + _dbus_mutex_new_at_location ((DBusMutex **) location_p, + thread_functions.recursive_mutex_new, + thread_functions.recursive_mutex_free, + &uninitialized_rmutex_list); +} + +/** + * Creates a new mutex using the function supplied to dbus_threads_init(), + * or creates a no-op mutex if threads are not initialized. + * May return #NULL even if threads are initialized, indicating + * out-of-memory. + * + * 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 real a real mutex so libraries @@ -84,65 +121,120 @@ _dbus_mutex_new (void) * @param location_p the location of the new mutex, can return #NULL on OOM */ void -_dbus_mutex_new_at_location (DBusMutex **location_p) +_dbus_cmutex_new_at_location (DBusCMutex **location_p) +{ + _dbus_mutex_new_at_location ((DBusMutex **) location_p, + thread_functions.mutex_new, + thread_functions.mutex_free, + &uninitialized_cmutex_list); +} + +static void +_dbus_mutex_new_at_location (DBusMutex **location_p, + DBusMutexNewFunction ctor, + DBusMutexFreeFunction dtor, + DBusList **uninitialized) { _dbus_assert (location_p != NULL); - *location_p = _dbus_mutex_new(); + *location_p = _dbus_mutex_new (ctor); if (thread_init_generation != _dbus_current_generation && *location_p) { - if (!_dbus_list_append (&uninitialized_mutex_list, location_p)) + if (!_dbus_list_append (uninitialized, location_p)) { - _dbus_mutex_free (*location_p); + _dbus_mutex_free (*location_p, dtor); *location_p = NULL; } } } static void -_dbus_mutex_free (DBusMutex *mutex) +_dbus_mutex_free (DBusMutex *mutex, + DBusMutexFreeFunction dtor) { if (mutex) { - if (mutex && thread_functions.recursive_mutex_free) + if (dtor) + dtor (mutex); + else 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); } } -/** - * Frees a mutex and removes it from the - * uninitialized_mutex_list; - * does nothing if passed a #NULL pointer. - */ -void -_dbus_mutex_free_at_location (DBusMutex **location_p) +static void +_dbus_mutex_free_at_location (DBusMutex **location_p, + DBusMutexFreeFunction dtor, + DBusList **uninitialized) { if (location_p) { if (thread_init_generation != _dbus_current_generation) - _dbus_list_remove (&uninitialized_mutex_list, location_p); + _dbus_list_remove (uninitialized, location_p); - _dbus_mutex_free (*location_p); + _dbus_mutex_free (*location_p, dtor); } } /** + * Frees a DBusRMutex and removes it from the + * uninitialized mutex list; + * does nothing if passed a #NULL pointer. + */ +void +_dbus_rmutex_free_at_location (DBusRMutex **location_p) +{ + _dbus_mutex_free_at_location ((DBusMutex **) location_p, + thread_functions.recursive_mutex_free, + &uninitialized_rmutex_list); +} + +/** + * Frees a DBusCMutex and removes it from the + * uninitialized mutex list; + * does nothing if passed a #NULL pointer. + */ +void +_dbus_cmutex_free_at_location (DBusCMutex **location_p) +{ + _dbus_mutex_free_at_location ((DBusMutex **) location_p, + thread_functions.mutex_free, + &uninitialized_cmutex_list); +} + +/** * Locks a mutex. Does nothing if passed a #NULL pointer. * Locks may be recursive if threading implementation initialized * recursive locks. */ void -_dbus_mutex_lock (DBusMutex *mutex) +_dbus_rmutex_lock (DBusRMutex *mutex) { - if (mutex) + if (mutex) { if (thread_functions.recursive_mutex_lock) - (* thread_functions.recursive_mutex_lock) (mutex); + (* thread_functions.recursive_mutex_lock) ((DBusMutex *) mutex); else if (thread_functions.mutex_lock) - (* thread_functions.mutex_lock) (mutex); + (* thread_functions.mutex_lock) ((DBusMutex *) mutex); + } +} + +/** + * Locks a mutex. Does nothing if passed a #NULL pointer. + * Locks may be recursive if threading implementation initialized + * recursive locks. + */ +void +_dbus_cmutex_lock (DBusCMutex *mutex) +{ + if (mutex) + { + if (thread_functions.mutex_lock) + (* thread_functions.mutex_lock) ((DBusMutex *) mutex); + else if (thread_functions.recursive_mutex_lock) + (* thread_functions.recursive_mutex_lock) ((DBusMutex *) mutex); } } @@ -152,14 +244,31 @@ _dbus_mutex_lock (DBusMutex *mutex) * @returns #TRUE on success */ void -_dbus_mutex_unlock (DBusMutex *mutex) +_dbus_rmutex_unlock (DBusRMutex *mutex) { if (mutex) { if (thread_functions.recursive_mutex_unlock) - (* thread_functions.recursive_mutex_unlock) (mutex); + (* thread_functions.recursive_mutex_unlock) ((DBusMutex *) mutex); else if (thread_functions.mutex_unlock) - (* thread_functions.mutex_unlock) (mutex); + (* thread_functions.mutex_unlock) ((DBusMutex *) mutex); + } +} + +/** + * Unlocks a mutex. Does nothing if passed a #NULL pointer. + * + * @returns #TRUE on success + */ +void +_dbus_cmutex_unlock (DBusCMutex *mutex) +{ + if (mutex) + { + if (thread_functions.mutex_unlock) + (* thread_functions.mutex_unlock) ((DBusMutex *) mutex); + else if (thread_functions.recursive_mutex_unlock) + (* thread_functions.recursive_mutex_unlock) ((DBusMutex *) mutex); } } @@ -243,10 +352,10 @@ _dbus_condvar_free_at_location (DBusCondVar **location_p) */ void _dbus_condvar_wait (DBusCondVar *cond, - DBusMutex *mutex) + DBusCMutex *mutex) { if (cond && mutex && thread_functions.condvar_wait) - (* thread_functions.condvar_wait) (cond, mutex); + (* thread_functions.condvar_wait) (cond, (DBusMutex *) mutex); } /** @@ -262,11 +371,13 @@ _dbus_condvar_wait (DBusCondVar *cond, */ dbus_bool_t _dbus_condvar_wait_timeout (DBusCondVar *cond, - DBusMutex *mutex, + DBusCMutex *mutex, int timeout_milliseconds) { if (cond && mutex && thread_functions.condvar_wait) - return (* thread_functions.condvar_wait_timeout) (cond, mutex, timeout_milliseconds); + return (* thread_functions.condvar_wait_timeout) (cond, + (DBusMutex *) mutex, + timeout_milliseconds); else return TRUE; } @@ -304,7 +415,7 @@ shutdown_global_locks (void *data) i = 0; while (i < _DBUS_N_GLOBAL_LOCKS) { - _dbus_mutex_free (*(locks[i])); + _dbus_mutex_free (*(locks[i]), thread_functions.recursive_mutex_free); *(locks[i]) = NULL; ++i; } @@ -315,7 +426,8 @@ shutdown_global_locks (void *data) static void shutdown_uninitialized_locks (void *data) { - _dbus_list_clear (&uninitialized_mutex_list); + _dbus_list_clear (&uninitialized_rmutex_list); + _dbus_list_clear (&uninitialized_cmutex_list); _dbus_list_clear (&uninitialized_condvar_list); } @@ -326,19 +438,34 @@ init_uninitialized_locks (void) _dbus_assert (thread_init_generation != _dbus_current_generation); - link = uninitialized_mutex_list; + link = uninitialized_rmutex_list; + while (link != NULL) + { + DBusMutex **mp; + + mp = link->data; + _dbus_assert (*mp == _DBUS_DUMMY_MUTEX); + + *mp = _dbus_mutex_new (thread_functions.recursive_mutex_new); + if (*mp == NULL) + goto fail_mutex; + + link = _dbus_list_get_next_link (&uninitialized_rmutex_list, link); + } + + link = uninitialized_cmutex_list; while (link != NULL) { DBusMutex **mp; - mp = (DBusMutex **)link->data; + mp = link->data; _dbus_assert (*mp == _DBUS_DUMMY_MUTEX); - *mp = _dbus_mutex_new (); + *mp = _dbus_mutex_new (thread_functions.mutex_new); if (*mp == NULL) goto fail_mutex; - link = _dbus_list_get_next_link (&uninitialized_mutex_list, link); + link = _dbus_list_get_next_link (&uninitialized_cmutex_list, link); } link = uninitialized_condvar_list; @@ -356,7 +483,8 @@ init_uninitialized_locks (void) link = _dbus_list_get_next_link (&uninitialized_condvar_list, link); } - _dbus_list_clear (&uninitialized_mutex_list); + _dbus_list_clear (&uninitialized_rmutex_list); + _dbus_list_clear (&uninitialized_cmutex_list); _dbus_list_clear (&uninitialized_condvar_list); if (!_dbus_register_shutdown_func (shutdown_uninitialized_locks, @@ -384,21 +512,38 @@ init_uninitialized_locks (void) } fail_mutex: - link = uninitialized_mutex_list; + link = uninitialized_rmutex_list; while (link != NULL) { DBusMutex **mp; - mp = (DBusMutex **)link->data; + mp = link->data; if (*mp != _DBUS_DUMMY_MUTEX) - _dbus_mutex_free (*mp); + _dbus_mutex_free (*mp, thread_functions.recursive_mutex_free); else break; *mp = _DBUS_DUMMY_MUTEX; - link = _dbus_list_get_next_link (&uninitialized_mutex_list, link); + link = _dbus_list_get_next_link (&uninitialized_rmutex_list, link); + } + + link = uninitialized_cmutex_list; + while (link != NULL) + { + DBusMutex **mp; + + mp = link->data; + + if (*mp != _DBUS_DUMMY_MUTEX) + _dbus_mutex_free (*mp, thread_functions.mutex_free); + else + break; + + *mp = _DBUS_DUMMY_MUTEX; + + link = _dbus_list_get_next_link (&uninitialized_cmutex_list, link); } return FALSE; @@ -408,9 +553,8 @@ static dbus_bool_t init_locks (void) { int i; - DBusMutex ***dynamic_global_locks; - - DBusMutex **global_locks[] = { + DBusRMutex ***dynamic_global_locks; + DBusRMutex **global_locks[] = { #define LOCK_ADDR(name) (& _dbus_lock_##name) LOCK_ADDR (win_fds), LOCK_ADDR (sid_atom_cache), @@ -437,14 +581,14 @@ init_locks (void) i = 0; - dynamic_global_locks = dbus_new (DBusMutex**, _DBUS_N_GLOBAL_LOCKS); + dynamic_global_locks = dbus_new (DBusRMutex**, _DBUS_N_GLOBAL_LOCKS); if (dynamic_global_locks == NULL) goto failed; while (i < _DBUS_N_ELEMENTS (global_locks)) { - *global_locks[i] = _dbus_mutex_new (); - + *global_locks[i] = (DBusRMutex *) _dbus_mutex_new (thread_functions.recursive_mutex_new); + if (*global_locks[i] == NULL) goto failed; @@ -467,7 +611,8 @@ init_locks (void) for (i = i - 1; i >= 0; i--) { - _dbus_mutex_free (*global_locks[i]); + _dbus_mutex_free ((DBusMutex *) *global_locks[i], + thread_functions.recursive_mutex_free); *global_locks[i] = NULL; } return FALSE; -- 1.7.9