From cc73b3da32ff6d4bebe9013b812f2845ad282cf7 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Thu, 24 Feb 2005 18:37:16 +0000 Subject: 2005-02-24 Havoc Pennington * dbus/dbus-server.c, dbus/dbus-server-unix.c: change semantics so you must disconnect before unref, since locking and other things are screwed up otherwise. Fix assorted other locking stuff. * dbus/dbus-signature.c (dbus_signature_iter_get_element_type): fix compilation * dbus/dbus-threads-internal.h: move the mutex/condvar wrappers into a private header and don't export from the library * throughout - call _dbus_thread_stuff vs. dbus_thread_stuff --- ChangeLog | 14 +++++++++ dbus/Makefile.am | 1 + dbus/dbus-connection.c | 69 ++++++++++++++++++++++---------------------- dbus/dbus-dataslot.c | 24 +++++++-------- dbus/dbus-internals.h | 4 +-- dbus/dbus-server-protected.h | 5 ++-- dbus/dbus-server-unix.c | 29 ++++++++++++++----- dbus/dbus-server.c | 62 +++++++++++++++++++++++++++------------ dbus/dbus-signature.c | 2 +- dbus/dbus-threads.c | 51 ++++++++++++++++++++------------ dbus/dbus-threads.h | 18 ------------ test/glib/test-profile.c | 5 ++-- 12 files changed, 167 insertions(+), 117 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5359bf51..9ff8aa44 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2005-02-24 Havoc Pennington + + * dbus/dbus-server.c, dbus/dbus-server-unix.c: change semantics so + you must disconnect before unref, since locking and other things + are screwed up otherwise. Fix assorted other locking stuff. + + * dbus/dbus-signature.c (dbus_signature_iter_get_element_type): + fix compilation + + * dbus/dbus-threads-internal.h: move the mutex/condvar wrappers + into a private header and don't export from the library + + * throughout - call _dbus_thread_stuff vs. dbus_thread_stuff + 2005-02-24 Colin Walters * dbus/dbus-signature.c: New file; implements various functions diff --git a/dbus/Makefile.am b/dbus/Makefile.am index 24d99107..ed58cd75 100644 --- a/dbus/Makefile.am +++ b/dbus/Makefile.am @@ -82,6 +82,7 @@ DBUS_LIB_SOURCES= \ dbus-signature.h \ dbus-timeout.c \ dbus-timeout.h \ + dbus-threads-internal.h \ dbus-threads.c \ dbus-transport.c \ dbus-transport.h \ diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index ae83af69..9c57edc9 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -38,6 +38,7 @@ #include "dbus-string.h" #include "dbus-pending-call.h" #include "dbus-object-tree.h" +#include "dbus-threads-internal.h" #ifdef DBUS_DISABLE_CHECKS #define TOOK_LOCK_CHECK(connection) @@ -60,14 +61,14 @@ #define CONNECTION_LOCK(connection) do { \ if (TRACE_LOCKS) { _dbus_verbose (" LOCK: %s\n", _DBUS_FUNCTION_NAME); } \ - dbus_mutex_lock ((connection)->mutex); \ + _dbus_mutex_lock ((connection)->mutex); \ TOOK_LOCK_CHECK (connection); \ } while (0) #define CONNECTION_UNLOCK(connection) do { \ if (TRACE_LOCKS) { _dbus_verbose (" UNLOCK: %s\n", _DBUS_FUNCTION_NAME); } \ RELEASING_LOCK_CHECK (connection); \ - dbus_mutex_unlock ((connection)->mutex); \ + _dbus_mutex_unlock ((connection)->mutex); \ } while (0) #define DISPATCH_STATUS_NAME(s) \ @@ -922,7 +923,7 @@ _dbus_connection_acquire_io_path (DBusConnection *connection, CONNECTION_UNLOCK (connection); _dbus_verbose ("%s locking io_path_mutex\n", _DBUS_FUNCTION_NAME); - dbus_mutex_lock (connection->io_path_mutex); + _dbus_mutex_lock (connection->io_path_mutex); _dbus_verbose ("%s start connection->io_path_acquired = %d timeout = %d\n", _DBUS_FUNCTION_NAME, connection->io_path_acquired, timeout_milliseconds); @@ -935,16 +936,16 @@ _dbus_connection_acquire_io_path (DBusConnection *connection, { _dbus_verbose ("%s waiting %d for IO path to be acquirable\n", _DBUS_FUNCTION_NAME, timeout_milliseconds); - dbus_condvar_wait_timeout (connection->io_path_cond, - connection->io_path_mutex, - timeout_milliseconds); + _dbus_condvar_wait_timeout (connection->io_path_cond, + connection->io_path_mutex, + timeout_milliseconds); } else { while (connection->io_path_acquired) { _dbus_verbose ("%s waiting for IO path to be acquirable\n", _DBUS_FUNCTION_NAME); - dbus_condvar_wait (connection->io_path_cond, connection->io_path_mutex); + _dbus_condvar_wait (connection->io_path_cond, connection->io_path_mutex); } } } @@ -959,7 +960,7 @@ _dbus_connection_acquire_io_path (DBusConnection *connection, _DBUS_FUNCTION_NAME, connection->io_path_acquired, we_acquired); _dbus_verbose ("%s unlocking io_path_mutex\n", _DBUS_FUNCTION_NAME); - dbus_mutex_unlock (connection->io_path_mutex); + _dbus_mutex_unlock (connection->io_path_mutex); CONNECTION_LOCK (connection); @@ -983,7 +984,7 @@ _dbus_connection_release_io_path (DBusConnection *connection) HAVE_LOCK_CHECK (connection); _dbus_verbose ("%s locking io_path_mutex\n", _DBUS_FUNCTION_NAME); - dbus_mutex_lock (connection->io_path_mutex); + _dbus_mutex_lock (connection->io_path_mutex); _dbus_assert (connection->io_path_acquired); @@ -991,10 +992,10 @@ _dbus_connection_release_io_path (DBusConnection *connection) _DBUS_FUNCTION_NAME, connection->io_path_acquired); connection->io_path_acquired = FALSE; - dbus_condvar_wake_one (connection->io_path_cond); + _dbus_condvar_wake_one (connection->io_path_cond); _dbus_verbose ("%s unlocking io_path_mutex\n", _DBUS_FUNCTION_NAME); - dbus_mutex_unlock (connection->io_path_mutex); + _dbus_mutex_unlock (connection->io_path_mutex); } /** @@ -1113,27 +1114,27 @@ _dbus_connection_new_for_transport (DBusTransport *transport) if (connection == NULL) goto error; - mutex = dbus_mutex_new (); + mutex = _dbus_mutex_new (); if (mutex == NULL) goto error; - io_path_mutex = dbus_mutex_new (); + io_path_mutex = _dbus_mutex_new (); if (io_path_mutex == NULL) goto error; - dispatch_mutex = dbus_mutex_new (); + dispatch_mutex = _dbus_mutex_new (); if (dispatch_mutex == NULL) goto error; - message_returned_cond = dbus_condvar_new (); + message_returned_cond = _dbus_condvar_new (); if (message_returned_cond == NULL) goto error; - dispatch_cond = dbus_condvar_new (); + dispatch_cond = _dbus_condvar_new (); if (dispatch_cond == NULL) goto error; - io_path_cond = dbus_condvar_new (); + io_path_cond = _dbus_condvar_new (); if (io_path_cond == NULL) goto error; @@ -1203,22 +1204,22 @@ _dbus_connection_new_for_transport (DBusTransport *transport) _dbus_list_free_link (disconnect_link); if (io_path_cond != NULL) - dbus_condvar_free (io_path_cond); + _dbus_condvar_free (io_path_cond); if (dispatch_cond != NULL) - dbus_condvar_free (dispatch_cond); + _dbus_condvar_free (dispatch_cond); if (message_returned_cond != NULL) - dbus_condvar_free (message_returned_cond); + _dbus_condvar_free (message_returned_cond); if (mutex != NULL) - dbus_mutex_free (mutex); + _dbus_mutex_free (mutex); if (io_path_mutex != NULL) - dbus_mutex_free (io_path_mutex); + _dbus_mutex_free (io_path_mutex); if (dispatch_mutex != NULL) - dbus_mutex_free (dispatch_mutex); + _dbus_mutex_free (dispatch_mutex); if (connection != NULL) dbus_free (connection); @@ -1540,13 +1541,13 @@ _dbus_connection_last_unref (DBusConnection *connection) _dbus_list_clear (&connection->link_cache); - dbus_condvar_free (connection->dispatch_cond); - dbus_condvar_free (connection->io_path_cond); + _dbus_condvar_free (connection->dispatch_cond); + _dbus_condvar_free (connection->io_path_cond); - dbus_mutex_free (connection->io_path_mutex); - dbus_mutex_free (connection->dispatch_mutex); + _dbus_mutex_free (connection->io_path_mutex); + _dbus_mutex_free (connection->dispatch_mutex); - dbus_mutex_free (connection->mutex); + _dbus_mutex_free (connection->mutex); dbus_free (connection); } @@ -2758,12 +2759,12 @@ _dbus_connection_acquire_dispatch (DBusConnection *connection) CONNECTION_UNLOCK (connection); _dbus_verbose ("%s locking dispatch_mutex\n", _DBUS_FUNCTION_NAME); - dbus_mutex_lock (connection->dispatch_mutex); + _dbus_mutex_lock (connection->dispatch_mutex); while (connection->dispatch_acquired) { _dbus_verbose ("%s waiting for dispatch to be acquirable\n", _DBUS_FUNCTION_NAME); - dbus_condvar_wait (connection->dispatch_cond, connection->dispatch_mutex); + _dbus_condvar_wait (connection->dispatch_cond, connection->dispatch_mutex); } _dbus_assert (!connection->dispatch_acquired); @@ -2771,7 +2772,7 @@ _dbus_connection_acquire_dispatch (DBusConnection *connection) connection->dispatch_acquired = TRUE; _dbus_verbose ("%s unlocking dispatch_mutex\n", _DBUS_FUNCTION_NAME); - dbus_mutex_unlock (connection->dispatch_mutex); + _dbus_mutex_unlock (connection->dispatch_mutex); CONNECTION_LOCK (connection); _dbus_connection_unref_unlocked (connection); @@ -2790,15 +2791,15 @@ _dbus_connection_release_dispatch (DBusConnection *connection) HAVE_LOCK_CHECK (connection); _dbus_verbose ("%s locking dispatch_mutex\n", _DBUS_FUNCTION_NAME); - dbus_mutex_lock (connection->dispatch_mutex); + _dbus_mutex_lock (connection->dispatch_mutex); _dbus_assert (connection->dispatch_acquired); connection->dispatch_acquired = FALSE; - dbus_condvar_wake_one (connection->dispatch_cond); + _dbus_condvar_wake_one (connection->dispatch_cond); _dbus_verbose ("%s unlocking dispatch_mutex\n", _DBUS_FUNCTION_NAME); - dbus_mutex_unlock (connection->dispatch_mutex); + _dbus_mutex_unlock (connection->dispatch_mutex); } static void diff --git a/dbus/dbus-dataslot.c b/dbus/dbus-dataslot.c index 675f5cf6..8026d048 100644 --- a/dbus/dbus-dataslot.c +++ b/dbus/dbus-dataslot.c @@ -70,7 +70,7 @@ _dbus_data_slot_allocator_alloc (DBusDataSlotAllocator *allocator, { dbus_int32_t slot; - if (!dbus_mutex_lock (mutex)) + if (!_dbus_mutex_lock (mutex)) return FALSE; if (allocator->n_allocated_slots == 0) @@ -142,7 +142,7 @@ _dbus_data_slot_allocator_alloc (DBusDataSlotAllocator *allocator, slot, allocator, allocator->n_allocated_slots, allocator->n_used_slots); out: - dbus_mutex_unlock (allocator->lock); + _dbus_mutex_unlock (allocator->lock); return slot >= 0; } @@ -161,7 +161,7 @@ void _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator, dbus_int32_t *slot_id_p) { - dbus_mutex_lock (allocator->lock); + _dbus_mutex_lock (allocator->lock); _dbus_assert (*slot_id_p < allocator->n_allocated_slots); _dbus_assert (allocator->allocated_slots[*slot_id_p].slot_id == *slot_id_p); @@ -171,7 +171,7 @@ _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator, if (allocator->allocated_slots[*slot_id_p].refcount > 0) { - dbus_mutex_unlock (allocator->lock); + _dbus_mutex_unlock (allocator->lock); return; } @@ -193,11 +193,11 @@ _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator, allocator->n_allocated_slots = 0; allocator->lock = NULL; - dbus_mutex_unlock (mutex); + _dbus_mutex_unlock (mutex); } else { - dbus_mutex_unlock (allocator->lock); + _dbus_mutex_unlock (allocator->lock); } } @@ -243,11 +243,11 @@ _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)) + if (!_dbus_mutex_lock (allocator->lock)) return FALSE; _dbus_assert (slot < allocator->n_allocated_slots); _dbus_assert (allocator->allocated_slots[slot].slot_id == slot); - dbus_mutex_unlock (allocator->lock); + _dbus_mutex_unlock (allocator->lock); #endif if (slot >= list->n_slots) @@ -301,12 +301,12 @@ _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)) + if (!_dbus_mutex_lock (allocator->lock)) return FALSE; _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); + _dbus_mutex_unlock (allocator->lock); #endif if (slot >= list->n_slots) @@ -389,7 +389,7 @@ _dbus_data_slot_test (void) _dbus_data_slot_list_init (&list); - mutex = dbus_mutex_new (); + mutex = _dbus_mutex_new (); if (mutex == NULL) _dbus_assert_not_reached ("failed to alloc mutex"); @@ -469,7 +469,7 @@ _dbus_data_slot_test (void) ++i; } - dbus_mutex_free (mutex); + _dbus_mutex_free (mutex); return TRUE; } diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index be19e1bc..dcbc6183 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -267,8 +267,8 @@ extern int _dbus_current_generation; #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_LOCK(name) _dbus_mutex_lock (_dbus_lock_##name) +#define _DBUS_UNLOCK(name) _dbus_mutex_unlock (_dbus_lock_##name) _DBUS_DECLARE_GLOBAL_LOCK (list); _DBUS_DECLARE_GLOBAL_LOCK (connection_slots); diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h index 5862c6c2..284d02c5 100644 --- a/dbus/dbus-server-protected.h +++ b/dbus/dbus-server-protected.h @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -125,14 +126,14 @@ void _dbus_server_unref_unlocked (DBusServer *server); #define SERVER_LOCK(server) do { \ if (TRACE_LOCKS) { _dbus_verbose (" LOCK: %s\n", _DBUS_FUNCTION_NAME); } \ - dbus_mutex_lock ((server)->mutex); \ + _dbus_mutex_lock ((server)->mutex); \ TOOK_LOCK_CHECK (server); \ } while (0) #define SERVER_UNLOCK(server) do { \ if (TRACE_LOCKS) { _dbus_verbose (" UNLOCK: %s\n", _DBUS_FUNCTION_NAME); } \ RELEASING_LOCK_CHECK (server); \ - dbus_mutex_unlock ((server)->mutex); \ + _dbus_mutex_unlock ((server)->mutex); \ } while (0) DBUS_END_DECLS diff --git a/dbus/dbus-server-unix.c b/dbus/dbus-server-unix.c index 4a07f149..0aef536e 100644 --- a/dbus/dbus-server-unix.c +++ b/dbus/dbus-server-unix.c @@ -61,6 +61,12 @@ unix_finalize (DBusServer *server) _dbus_server_finalize_base (server); + if (unix_server->watch) + { + _dbus_watch_unref (unix_server->watch); + unix_server->watch = NULL; + } + dbus_free (unix_server->socket_name); dbus_free (server); } @@ -69,6 +75,9 @@ unix_finalize (DBusServer *server) * @todo unreffing the connection at the end may cause * us to drop the last ref to the connection before * disconnecting it. That is invalid. + * + * @todo doesn't this leak a server refcount if + * new_connection_function is NULL? */ /* Return value is just for memory, not other failures. */ static dbus_bool_t @@ -200,6 +209,8 @@ unix_disconnect (DBusServer *server) { DBusServerUnix *unix_server = (DBusServerUnix*) server; + HAVE_LOCK_CHECK (server); + if (unix_server->watch) { _dbus_server_remove_watch (server, @@ -217,6 +228,8 @@ unix_disconnect (DBusServer *server) _dbus_string_init_const (&tmp, unix_server->socket_name); _dbus_delete_file (&tmp, NULL); } + + HAVE_LOCK_CHECK (server); } static DBusServerVTable unix_vtable = { @@ -242,12 +255,13 @@ _dbus_server_new_for_fd (int fd, const DBusString *address) { DBusServerUnix *unix_server; + DBusServer *server; DBusWatch *watch; unix_server = dbus_new0 (DBusServerUnix, 1); if (unix_server == NULL) return NULL; - + watch = _dbus_watch_new (fd, DBUS_WATCH_READABLE, TRUE, @@ -267,26 +281,25 @@ _dbus_server_new_for_fd (int fd, return NULL; } -#ifndef DBUS_DISABLE_CHECKS - unix_server->base.have_server_lock = TRUE; -#endif + server = (DBusServer*) unix_server; + + SERVER_LOCK (server); if (!_dbus_server_add_watch (&unix_server->base, watch)) { + SERVER_UNLOCK (server); _dbus_server_finalize_base (&unix_server->base); _dbus_watch_unref (watch); dbus_free (unix_server); return NULL; } - -#ifndef DBUS_DISABLE_CHECKS - unix_server->base.have_server_lock = FALSE; -#endif unix_server->fd = fd; unix_server->watch = watch; + SERVER_UNLOCK (server); + return (DBusServer*) unix_server; } diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c index c11fbd48..156d5ccd 100644 --- a/dbus/dbus-server.c +++ b/dbus/dbus-server.c @@ -76,7 +76,7 @@ _dbus_server_init_base (DBusServer *server, if (!_dbus_string_copy_data (address, &server->address)) goto failed; - server->mutex = dbus_mutex_new (); + server->mutex = _dbus_mutex_new (); if (server->mutex == NULL) goto failed; @@ -97,7 +97,7 @@ _dbus_server_init_base (DBusServer *server, failed: if (server->mutex) { - dbus_mutex_free (server->mutex); + _dbus_mutex_free (server->mutex); server->mutex = NULL; } if (server->watches) @@ -127,19 +127,24 @@ _dbus_server_init_base (DBusServer *server, */ void _dbus_server_finalize_base (DBusServer *server) -{ +{ + /* We don't have the lock, but nobody should be accessing + * concurrently since they don't have a ref + */ +#ifndef DBUS_DISABLE_CHECKS + _dbus_assert (!server->have_server_lock); +#endif + _dbus_assert (server->disconnected); + /* calls out to application code... */ _dbus_data_slot_list_free (&server->slot_list); dbus_server_set_new_connection_function (server, NULL, NULL, NULL); - if (!server->disconnected) - dbus_server_disconnect (server); - _dbus_watch_list_free (server->watches); _dbus_timeout_list_free (server->timeouts); - dbus_mutex_free (server->mutex); + _dbus_mutex_free (server->mutex); dbus_free (server->address); @@ -168,8 +173,9 @@ protected_change_watch (DBusServer *server, HAVE_LOCK_CHECK (server); - /* This isn't really safe or reasonable; a better pattern is the "do everything, then - * drop lock and call out" one; but it has to be propagated up through all callers + /* This isn't really safe or reasonable; a better pattern is the "do + * everything, then drop lock and call out" one; but it has to be + * propagated up through all callers */ watches = server->watches; @@ -213,6 +219,7 @@ dbus_bool_t _dbus_server_add_watch (DBusServer *server, DBusWatch *watch) { + HAVE_LOCK_CHECK (server); return protected_change_watch (server, watch, _dbus_watch_list_add_watch, NULL, NULL, FALSE); @@ -228,6 +235,7 @@ void _dbus_server_remove_watch (DBusServer *server, DBusWatch *watch) { + HAVE_LOCK_CHECK (server); protected_change_watch (server, watch, NULL, _dbus_watch_list_remove_watch, @@ -250,6 +258,7 @@ _dbus_server_toggle_watch (DBusServer *server, { _dbus_assert (watch != NULL); + HAVE_LOCK_CHECK (server); protected_change_watch (server, watch, NULL, NULL, _dbus_watch_list_toggle_watch, @@ -595,6 +604,7 @@ DBusServer * dbus_server_ref (DBusServer *server) { _dbus_return_val_if_fail (server != NULL, NULL); + _dbus_return_val_if_fail (server->refcount.value > 0, NULL); #ifdef DBUS_HAVE_ATOMIC_INT _dbus_atomic_inc (&server->refcount); @@ -611,9 +621,9 @@ dbus_server_ref (DBusServer *server) /** * Decrements the reference count of a DBusServer. Finalizes the - * server if the reference count reaches zero. The server connection - * will be closed as with dbus_server_disconnect() when the server is - * finalized. + * server if the reference count reaches zero. + * + * The server must be disconnected before the refcount reaches zero. * * @param server the server. */ @@ -623,6 +633,7 @@ dbus_server_unref (DBusServer *server) dbus_bool_t last_unref; _dbus_return_if_fail (server != NULL); + _dbus_return_if_fail (server->refcount.value > 0); #ifdef DBUS_HAVE_ATOMIC_INT last_unref = (_dbus_atomic_dec (&server->refcount) == 1); @@ -639,6 +650,9 @@ dbus_server_unref (DBusServer *server) if (last_unref) { + /* lock not held! */ + _dbus_assert (server->disconnected); + _dbus_assert (server->vtable->finalize != NULL); (* server->vtable->finalize) (server); @@ -653,6 +667,9 @@ dbus_server_unref (DBusServer *server) void _dbus_server_ref_unlocked (DBusServer *server) { + _dbus_assert (server != NULL); + _dbus_assert (server->refcount.value > 0); + HAVE_LOCK_CHECK (server); #ifdef DBUS_HAVE_ATOMIC_INT @@ -675,6 +692,7 @@ _dbus_server_unref_unlocked (DBusServer *server) dbus_bool_t last_unref; _dbus_assert (server != NULL); + _dbus_assert (server->refcount.value > 0); HAVE_LOCK_CHECK (server); @@ -689,6 +707,10 @@ _dbus_server_unref_unlocked (DBusServer *server) if (last_unref) { + _dbus_assert (server->disconnected); + + SERVER_UNLOCK (server); + _dbus_assert (server->vtable->finalize != NULL); (* server->vtable->finalize) (server); @@ -707,21 +729,23 @@ void dbus_server_disconnect (DBusServer *server) { _dbus_return_if_fail (server != NULL); + _dbus_return_if_fail (server->refcount.value > 0); SERVER_LOCK (server); + _dbus_server_ref_unlocked (server); _dbus_assert (server->vtable->disconnect != NULL); - if (server->disconnected) + if (!server->disconnected) { - SERVER_UNLOCK (server); - return; + /* this has to be first so recursive calls to disconnect don't happen */ + server->disconnected = TRUE; + + (* server->vtable->disconnect) (server); } - - (* server->vtable->disconnect) (server); - server->disconnected = TRUE; SERVER_UNLOCK (server); + dbus_server_unref (server); } /** @@ -1086,6 +1110,7 @@ _dbus_server_test (void) if (server == NULL) _dbus_assert_not_reached ("Failed to listen for valid address."); + dbus_server_disconnect (server); dbus_server_unref (server); /* Try disconnecting before unreffing */ @@ -1094,7 +1119,6 @@ _dbus_server_test (void) _dbus_assert_not_reached ("Failed to listen for valid address."); dbus_server_disconnect (server); - dbus_server_unref (server); } diff --git a/dbus/dbus-signature.c b/dbus/dbus-signature.c index c984bcfd..698c21ed 100644 --- a/dbus/dbus-signature.c +++ b/dbus/dbus-signature.c @@ -97,7 +97,7 @@ dbus_signature_iter_get_element_type (const DBusSignatureIter *iter) { DBusSignatureRealIter *real_iter = (DBusSignatureRealIter *) iter; - _dbus_retur(dbus_signature_iter_get_current_type (iter) == DBUS_TYPE_ARRAY); + _dbus_return_val_if_fail (dbus_signature_iter_get_current_type (iter) == DBUS_TYPE_ARRAY, DBUS_TYPE_INVALID); return _dbus_first_type_in_signature_c_str (real_iter->pos, 1); } diff --git a/dbus/dbus-threads.c b/dbus/dbus-threads.c index 3d1de5ef..0d004d62 100644 --- a/dbus/dbus-threads.c +++ b/dbus/dbus-threads.c @@ -22,6 +22,7 @@ */ #include "dbus-threads.h" #include "dbus-internals.h" +#include "dbus-threads-internal.h" static DBusThreadFunctions thread_functions = { @@ -41,9 +42,9 @@ static int thread_init_generation = 0; #define _DBUS_DUMMY_CONDVAR ((DBusCondVar*)0xABCDEF2) /** - * @defgroup DBusThreads Thread functions - * @ingroup DBus - * @brief dbus_threads_init(), dbus_mutex_lock(), etc. + * @defgroup DBusThreadsInternals Thread functions + * @ingroup DBusInternals + * @brief _dbus_mutex_lock(), etc. * * Functions and macros related to threads and thread locks. * @@ -59,7 +60,7 @@ static int thread_init_generation = 0; * @returns new mutex or #NULL */ DBusMutex* -dbus_mutex_new (void) +_dbus_mutex_new (void) { if (thread_functions.mutex_new) return (* thread_functions.mutex_new) (); @@ -72,7 +73,7 @@ dbus_mutex_new (void) * nothing if passed a #NULL pointer. */ void -dbus_mutex_free (DBusMutex *mutex) +_dbus_mutex_free (DBusMutex *mutex) { if (mutex && thread_functions.mutex_free) (* thread_functions.mutex_free) (mutex); @@ -85,7 +86,7 @@ dbus_mutex_free (DBusMutex *mutex) * @returns #TRUE on success */ dbus_bool_t -dbus_mutex_lock (DBusMutex *mutex) +_dbus_mutex_lock (DBusMutex *mutex) { if (mutex && thread_functions.mutex_lock) return (* thread_functions.mutex_lock) (mutex); @@ -99,7 +100,7 @@ dbus_mutex_lock (DBusMutex *mutex) * @returns #TRUE on success */ dbus_bool_t -dbus_mutex_unlock (DBusMutex *mutex) +_dbus_mutex_unlock (DBusMutex *mutex) { if (mutex && thread_functions.mutex_unlock) return (* thread_functions.mutex_unlock) (mutex); @@ -116,7 +117,7 @@ dbus_mutex_unlock (DBusMutex *mutex) * @returns new mutex or #NULL */ DBusCondVar * -dbus_condvar_new (void) +_dbus_condvar_new (void) { if (thread_functions.condvar_new) return (* thread_functions.condvar_new) (); @@ -129,7 +130,7 @@ dbus_condvar_new (void) * nothing if passed a #NULL pointer. */ void -dbus_condvar_free (DBusCondVar *cond) +_dbus_condvar_free (DBusCondVar *cond) { if (cond && thread_functions.condvar_free) (* thread_functions.condvar_free) (cond); @@ -142,8 +143,8 @@ dbus_condvar_free (DBusCondVar *cond) * Does nothing if passed a #NULL pointer. */ void -dbus_condvar_wait (DBusCondVar *cond, - DBusMutex *mutex) +_dbus_condvar_wait (DBusCondVar *cond, + DBusMutex *mutex) { if (cond && mutex && thread_functions.condvar_wait) (* thread_functions.condvar_wait) (cond, mutex); @@ -162,9 +163,9 @@ dbus_condvar_wait (DBusCondVar *cond, * timeout was reached. */ dbus_bool_t -dbus_condvar_wait_timeout (DBusCondVar *cond, - DBusMutex *mutex, - int timeout_milliseconds) +_dbus_condvar_wait_timeout (DBusCondVar *cond, + DBusMutex *mutex, + int timeout_milliseconds) { if (cond && mutex && thread_functions.condvar_wait) return (* thread_functions.condvar_wait_timeout) (cond, mutex, timeout_milliseconds); @@ -178,7 +179,7 @@ dbus_condvar_wait_timeout (DBusCondVar *cond, * Does nothing if passed a #NULL pointer. */ void -dbus_condvar_wake_one (DBusCondVar *cond) +_dbus_condvar_wake_one (DBusCondVar *cond) { if (cond && thread_functions.condvar_wake_one) (* thread_functions.condvar_wake_one) (cond); @@ -190,7 +191,7 @@ dbus_condvar_wake_one (DBusCondVar *cond) * Does nothing if passed a #NULL pointer. */ void -dbus_condvar_wake_all (DBusCondVar *cond) +_dbus_condvar_wake_all (DBusCondVar *cond) { if (cond && thread_functions.condvar_wake_all) (* thread_functions.condvar_wake_all) (cond); @@ -205,7 +206,7 @@ shutdown_global_locks (void *data) i = 0; while (i < _DBUS_N_GLOBAL_LOCKS) { - dbus_mutex_free (*(locks[i])); + _dbus_mutex_free (*(locks[i])); *(locks[i]) = NULL; ++i; } @@ -245,7 +246,7 @@ init_global_locks (void) while (i < _DBUS_N_ELEMENTS (global_locks)) { - *global_locks[i] = dbus_mutex_new (); + *global_locks[i] = _dbus_mutex_new (); if (*global_locks[i] == NULL) goto failed; @@ -266,14 +267,26 @@ init_global_locks (void) for (i = i - 1; i >= 0; i--) { - dbus_mutex_free (*global_locks[i]); + _dbus_mutex_free (*global_locks[i]); *global_locks[i] = NULL; } return FALSE; } +/** @} */ /* end of internals */ + +/** + * @defgroup DBusThreads Thread functions + * @ingroup DBus + * @brief dbus_threads_init() + * + * Functions and macros related to threads and thread locks. + * + * @{ + */ /** + * * 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 diff --git a/dbus/dbus-threads.h b/dbus/dbus-threads.h index 4391ff1d..cf1c01d2 100644 --- a/dbus/dbus-threads.h +++ b/dbus/dbus-threads.h @@ -97,26 +97,8 @@ typedef struct } DBusThreadFunctions; - -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); - -DBusCondVar* dbus_condvar_new (void); -void dbus_condvar_free (DBusCondVar *cond); -void dbus_condvar_wait (DBusCondVar *cond, - DBusMutex *mutex); -dbus_bool_t dbus_condvar_wait_timeout (DBusCondVar *cond, - DBusMutex *mutex, - int timeout_milliseconds); -void dbus_condvar_wake_one (DBusCondVar *cond); -void dbus_condvar_wake_all (DBusCondVar *cond); - dbus_bool_t dbus_threads_init (const DBusThreadFunctions *functions); - - DBUS_END_DECLS #endif /* DBUS_THREADS_H */ diff --git a/test/glib/test-profile.c b/test/glib/test-profile.c index ec0e4fd0..ca4f33c0 100644 --- a/test/glib/test-profile.c +++ b/test/glib/test-profile.c @@ -48,7 +48,7 @@ */ #define N_CLIENT_THREADS 1 /* It seems like at least 750000 or so iterations reduces the variability to sane levels */ -#define N_ITERATIONS 750 +#define N_ITERATIONS 750000 #define N_PROGRESS_UPDATES 20 /* Don't make PAYLOAD_SIZE too huge because it gets used as a static buffer size */ #define PAYLOAD_SIZE 0 @@ -303,8 +303,9 @@ no_bus_init_server (ServerData *sd) static void no_bus_stop_server (ServerData *sd, - void *server) + void *server) { + dbus_server_disconnect (server); dbus_server_unref (server); } -- cgit