diff options
Diffstat (limited to 'dbus')
| -rw-r--r-- | dbus/dbus-connection-internal.h | 2 | ||||
| -rw-r--r-- | dbus/dbus-connection.c | 461 | ||||
| -rw-r--r-- | dbus/dbus-object-tree.c | 45 | ||||
| -rw-r--r-- | dbus/dbus-server-protected.h | 39 | ||||
| -rw-r--r-- | dbus/dbus-server-unix.c | 55 | ||||
| -rw-r--r-- | dbus/dbus-server.c | 197 | ||||
| -rw-r--r-- | dbus/dbus-transport-unix.c | 18 | ||||
| -rw-r--r-- | dbus/dbus-transport.c | 59 |
8 files changed, 721 insertions, 155 deletions
diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h index 21dc415f..57e6fb0d 100644 --- a/dbus/dbus-connection-internal.h +++ b/dbus/dbus-connection-internal.h @@ -74,7 +74,7 @@ void _dbus_connection_toggle_timeout (DBusConnection DBusTimeout *timeout, dbus_bool_t enabled); DBusConnection* _dbus_connection_new_for_transport (DBusTransport *transport); -void _dbus_connection_do_iteration (DBusConnection *connection, +void _dbus_connection_do_iteration_unlocked (DBusConnection *connection, unsigned int flags, int timeout_milliseconds); diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index dfc5d44b..cac4fc8c 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -1,7 +1,7 @@ /* -*- mode: C; c-file-style: "gnu" -*- */ /* dbus-connection.c DBusConnection object * - * Copyright (C) 2002, 2003, 2004 Red Hat Inc. + * Copyright (C) 2002, 2003, 2004, 2005 Red Hat Inc. * * Licensed under the Academic Free License version 2.1 * @@ -39,20 +39,37 @@ #include "dbus-pending-call.h" #include "dbus-object-tree.h" -#if 0 -#define CONNECTION_LOCK(connection) do { \ - _dbus_verbose (" LOCK: %s\n", _DBUS_FUNCTION_NAME); \ - dbus_mutex_lock ((connection)->mutex); \ +#ifdef DBUS_DISABLE_CHECKS +#define TOOK_LOCK_CHECK(connection) +#define RELEASING_LOCK_CHECK(connection) +#define HAVE_LOCK_CHECK(connection) +#else +#define TOOK_LOCK_CHECK(connection) do { \ + _dbus_assert (!(connection)->have_connection_lock); \ + (connection)->have_connection_lock = TRUE; \ } while (0) -#define CONNECTION_UNLOCK(connection) do { \ - _dbus_verbose (" UNLOCK: %s\n", _DBUS_FUNCTION_NAME); \ - dbus_mutex_unlock ((connection)->mutex); \ +#define RELEASING_LOCK_CHECK(connection) do { \ + _dbus_assert ((connection)->have_connection_lock); \ + (connection)->have_connection_lock = FALSE; \ } while (0) -#else -#define CONNECTION_LOCK(connection) dbus_mutex_lock ((connection)->mutex) -#define CONNECTION_UNLOCK(connection) dbus_mutex_unlock ((connection)->mutex) +#define HAVE_LOCK_CHECK(connection) _dbus_assert ((connection)->have_connection_lock) +/* A "DO_NOT_HAVE_LOCK_CHECK" is impossible since we need the lock to check the flag */ #endif +#define TRACE_LOCKS 1 + +#define CONNECTION_LOCK(connection) do { \ + if (TRACE_LOCKS) { _dbus_verbose (" LOCK: %s\n", _DBUS_FUNCTION_NAME); } \ + 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); \ + } while (0) + #define DISPATCH_STATUS_NAME(s) \ ((s) == DBUS_DISPATCH_COMPLETE ? "complete" : \ (s) == DBUS_DISPATCH_DATA_REMAINS ? "data remains" : \ @@ -171,10 +188,7 @@ struct DBusConnection DBusMutex *mutex; /**< Lock on the entire DBusConnection */ - dbus_bool_t dispatch_acquired; /**< Protects dispatch() */ DBusCondVar *dispatch_cond; /**< Protects dispatch() */ - - dbus_bool_t io_path_acquired; /**< Protects transport io path */ DBusCondVar *io_path_cond; /**< Protects transport io path */ DBusList *outgoing_messages; /**< Queue of messages we need to send, send the end of the list first. */ @@ -215,9 +229,16 @@ struct DBusConnection * for the global linked list mempool lock */ DBusObjectTree *objects; /**< Object path handlers registered with this connection */ - + + unsigned int dispatch_acquired : 1; /**< Someone has dispatch path */ + unsigned int io_path_acquired : 1; /**< Someone has transport io path */ + unsigned int exit_on_disconnect : 1; /**< If #TRUE, exit after handling disconnect signal */ - + +#ifndef DBUS_DISABLE_CHECKS + unsigned int have_connection_lock : 1; /**< Used to check locking */ +#endif + #ifndef DBUS_DISABLE_CHECKS int generation; /**< _dbus_current_generation that should correspond to this connection */ #endif @@ -387,6 +408,8 @@ static void _dbus_connection_queue_synthesized_message_link (DBusConnection *connection, DBusList *link) { + HAVE_LOCK_CHECK (connection); + _dbus_list_append_link (&connection->incoming_messages, link); connection->n_incoming += 1; @@ -408,6 +431,7 @@ _dbus_connection_queue_synthesized_message_link (DBusConnection *connection, dbus_bool_t _dbus_connection_has_messages_to_send_unlocked (DBusConnection *connection) { + HAVE_LOCK_CHECK (connection); return connection->outgoing_messages != NULL; } @@ -441,6 +465,8 @@ dbus_connection_has_messages_to_send (DBusConnection *connection) DBusMessage* _dbus_connection_get_message_to_send (DBusConnection *connection) { + HAVE_LOCK_CHECK (connection); + return _dbus_list_get_last (&connection->outgoing_messages); } @@ -458,6 +484,8 @@ _dbus_connection_message_sent (DBusConnection *connection, { DBusList *link; + HAVE_LOCK_CHECK (connection); + /* This can be called before we even complete authentication, since * it's called on disconnect to clean up the outgoing queue. * It's also called as we successfully send each message. @@ -495,6 +523,62 @@ _dbus_connection_message_sent (DBusConnection *connection, dbus_message_unref (message); } +typedef dbus_bool_t (* DBusWatchAddFunction) (DBusWatchList *list, + DBusWatch *watch); +typedef void (* DBusWatchRemoveFunction) (DBusWatchList *list, + DBusWatch *watch); +typedef void (* DBusWatchToggleFunction) (DBusWatchList *list, + DBusWatch *watch, + dbus_bool_t enabled); + +static dbus_bool_t +protected_change_watch (DBusConnection *connection, + DBusWatch *watch, + DBusWatchAddFunction add_function, + DBusWatchRemoveFunction remove_function, + DBusWatchToggleFunction toggle_function, + dbus_bool_t enabled) +{ + DBusWatchList *watches; + dbus_bool_t retval; + + HAVE_LOCK_CHECK (connection); + + /* 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 = connection->watches; + if (watches) + { + connection->watches = NULL; + _dbus_connection_ref_unlocked (connection); + CONNECTION_UNLOCK (connection); + + if (add_function) + retval = (* add_function) (watches, watch); + else if (remove_function) + { + retval = TRUE; + (* remove_function) (watches, watch); + } + else + { + retval = TRUE; + (* toggle_function) (watches, watch, enabled); + } + + CONNECTION_LOCK (connection); + connection->watches = watches; + _dbus_connection_unref_unlocked (connection); + + return retval; + } + else + return FALSE; +} + + /** * Adds a watch using the connection's DBusAddWatchFunction if * available. Otherwise records the watch to be added when said @@ -509,11 +593,9 @@ dbus_bool_t _dbus_connection_add_watch (DBusConnection *connection, DBusWatch *watch) { - if (connection->watches) /* null during finalize */ - return _dbus_watch_list_add_watch (connection->watches, - watch); - else - return FALSE; + return protected_change_watch (connection, watch, + _dbus_watch_list_add_watch, + NULL, NULL, FALSE); } /** @@ -528,9 +610,10 @@ void _dbus_connection_remove_watch (DBusConnection *connection, DBusWatch *watch) { - if (connection->watches) /* null during finalize */ - _dbus_watch_list_remove_watch (connection->watches, - watch); + protected_change_watch (connection, watch, + NULL, + _dbus_watch_list_remove_watch, + NULL, FALSE); } /** @@ -549,10 +632,66 @@ _dbus_connection_toggle_watch (DBusConnection *connection, dbus_bool_t enabled) { _dbus_assert (watch != NULL); + + protected_change_watch (connection, watch, + NULL, NULL, + _dbus_watch_list_toggle_watch, + enabled); +} + +typedef dbus_bool_t (* DBusTimeoutAddFunction) (DBusTimeoutList *list, + DBusTimeout *timeout); +typedef void (* DBusTimeoutRemoveFunction) (DBusTimeoutList *list, + DBusTimeout *timeout); +typedef void (* DBusTimeoutToggleFunction) (DBusTimeoutList *list, + DBusTimeout *timeout, + dbus_bool_t enabled); + +static dbus_bool_t +protected_change_timeout (DBusConnection *connection, + DBusTimeout *timeout, + DBusTimeoutAddFunction add_function, + DBusTimeoutRemoveFunction remove_function, + DBusTimeoutToggleFunction toggle_function, + dbus_bool_t enabled) +{ + DBusTimeoutList *timeouts; + dbus_bool_t retval; + + HAVE_LOCK_CHECK (connection); + + /* 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 + */ - if (connection->watches) /* null during finalize */ - _dbus_watch_list_toggle_watch (connection->watches, - watch, enabled); + timeouts = connection->timeouts; + if (timeouts) + { + connection->timeouts = NULL; + _dbus_connection_ref_unlocked (connection); + CONNECTION_UNLOCK (connection); + + if (add_function) + retval = (* add_function) (timeouts, timeout); + else if (remove_function) + { + retval = TRUE; + (* remove_function) (timeouts, timeout); + } + else + { + retval = TRUE; + (* toggle_function) (timeouts, timeout, enabled); + } + + CONNECTION_LOCK (connection); + connection->timeouts = timeouts; + _dbus_connection_unref_unlocked (connection); + + return retval; + } + else + return FALSE; } /** @@ -570,11 +709,9 @@ dbus_bool_t _dbus_connection_add_timeout (DBusConnection *connection, DBusTimeout *timeout) { - if (connection->timeouts) /* null during finalize */ - return _dbus_timeout_list_add_timeout (connection->timeouts, - timeout); - else - return FALSE; + return protected_change_timeout (connection, timeout, + _dbus_timeout_list_add_timeout, + NULL, NULL, FALSE); } /** @@ -589,9 +726,10 @@ void _dbus_connection_remove_timeout (DBusConnection *connection, DBusTimeout *timeout) { - if (connection->timeouts) /* null during finalize */ - _dbus_timeout_list_remove_timeout (connection->timeouts, - timeout); + protected_change_timeout (connection, timeout, + NULL, + _dbus_timeout_list_remove_timeout, + NULL, FALSE); } /** @@ -604,19 +742,22 @@ _dbus_connection_remove_timeout (DBusConnection *connection, * @param enabled whether to enable or disable */ void -_dbus_connection_toggle_timeout (DBusConnection *connection, +_dbus_connection_toggle_timeout (DBusConnection *connection, DBusTimeout *timeout, - dbus_bool_t enabled) + dbus_bool_t enabled) { - if (connection->timeouts) /* null during finalize */ - _dbus_timeout_list_toggle_timeout (connection->timeouts, - timeout, enabled); + protected_change_timeout (connection, timeout, + NULL, NULL, + _dbus_timeout_list_toggle_timeout, + enabled); } static dbus_bool_t _dbus_connection_attach_pending_call_unlocked (DBusConnection *connection, DBusPendingCall *pending) { + HAVE_LOCK_CHECK (connection); + _dbus_assert (pending->reply_serial != 0); if (!_dbus_connection_add_timeout (connection, pending->timeout)) @@ -627,6 +768,8 @@ _dbus_connection_attach_pending_call_unlocked (DBusConnection *connection, pending)) { _dbus_connection_remove_timeout (connection, pending->timeout); + + HAVE_LOCK_CHECK (connection); return FALSE; } @@ -634,6 +777,8 @@ _dbus_connection_attach_pending_call_unlocked (DBusConnection *connection, pending->connection = connection; dbus_pending_call_ref (pending); + + HAVE_LOCK_CHECK (connection); return TRUE; } @@ -664,6 +809,19 @@ free_pending_call_on_hash_removal (void *data) } static void +_dbus_connection_detach_pending_call_unlocked (DBusConnection *connection, + DBusPendingCall *pending) +{ + /* Can't have a destroy notifier on the pending call if we're going to do this */ + + dbus_pending_call_ref (pending); + _dbus_hash_table_remove_int (connection->pending_replies, + pending->reply_serial); + _dbus_assert (pending->connection == NULL); + dbus_pending_call_unref (pending); +} + +static void _dbus_connection_detach_pending_call_and_unlock (DBusConnection *connection, DBusPendingCall *pending) { @@ -674,6 +832,7 @@ _dbus_connection_detach_pending_call_and_unlock (DBusConnection *connection, dbus_pending_call_ref (pending); _dbus_hash_table_remove_int (connection->pending_replies, pending->reply_serial); + _dbus_assert (pending->connection == NULL); CONNECTION_UNLOCK (connection); dbus_pending_call_unref (pending); } @@ -749,14 +908,25 @@ _dbus_connection_acquire_io_path (DBusConnection *connection, { dbus_bool_t res = TRUE; + _dbus_verbose ("%s start connection->io_path_acquired = %d timeout = %d\n", + _DBUS_FUNCTION_NAME, connection->io_path_acquired, timeout_milliseconds); + if (connection->io_path_acquired) { +#ifndef DBUS_DISABLE_CHECKS + connection->have_connection_lock = FALSE; +#endif + if (timeout_milliseconds != -1) res = dbus_condvar_wait_timeout (connection->io_path_cond, connection->mutex, timeout_milliseconds); else dbus_condvar_wait (connection->io_path_cond, connection->mutex); + +#ifndef DBUS_DISABLE_CHECKS + connection->have_connection_lock = TRUE; +#endif } if (res) @@ -765,6 +935,9 @@ _dbus_connection_acquire_io_path (DBusConnection *connection, connection->io_path_acquired = TRUE; } + + _dbus_verbose ("%s end connection->io_path_acquired = %d res = %d\n", + _DBUS_FUNCTION_NAME, connection->io_path_acquired, res); return res; } @@ -781,6 +954,9 @@ _dbus_connection_release_io_path (DBusConnection *connection) { _dbus_assert (connection->io_path_acquired); + _dbus_verbose ("%s start connection->io_path_acquired = %d\n", + _DBUS_FUNCTION_NAME, connection->io_path_acquired); + connection->io_path_acquired = FALSE; dbus_condvar_wake_one (connection->io_path_cond); } @@ -789,7 +965,7 @@ _dbus_connection_release_io_path (DBusConnection *connection) /** * Queues incoming messages and sends outgoing messages for this * connection, optionally blocking in the process. Each call to - * _dbus_connection_do_iteration() will call select() or poll() one + * _dbus_connection_do_iteration_unlocked() will call select() or poll() one * time and then read or write data if possible. * * The purpose of this function is to be able to flush outgoing @@ -815,10 +991,14 @@ _dbus_connection_release_io_path (DBusConnection *connection) * @param timeout_milliseconds maximum blocking time, or -1 for no limit. */ void -_dbus_connection_do_iteration (DBusConnection *connection, - unsigned int flags, - int timeout_milliseconds) +_dbus_connection_do_iteration_unlocked (DBusConnection *connection, + unsigned int flags, + int timeout_milliseconds) { + _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME); + + HAVE_LOCK_CHECK (connection); + if (connection->n_outgoing == 0) flags &= ~DBUS_ITERATION_DO_WRITING; @@ -829,6 +1009,8 @@ _dbus_connection_do_iteration (DBusConnection *connection, flags, timeout_milliseconds); _dbus_connection_release_io_path (connection); } + + _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME); } /** @@ -949,11 +1131,15 @@ _dbus_connection_new_for_transport (DBusTransport *transport) connection->client_serial = 1; connection->disconnect_message_link = disconnect_link; + + CONNECTION_LOCK (connection); if (!_dbus_transport_set_connection (transport, connection)) goto error; - _dbus_transport_ref (transport); + _dbus_transport_ref (transport); + + CONNECTION_UNLOCK (connection); return connection; @@ -1006,9 +1192,11 @@ _dbus_connection_new_for_transport (DBusTransport *transport) */ DBusConnection * _dbus_connection_ref_unlocked (DBusConnection *connection) -{ +{ _dbus_assert (connection != NULL); _dbus_assert (connection->generation == _dbus_current_generation); + + HAVE_LOCK_CHECK (connection); #ifdef DBUS_HAVE_ATOMIC_INT _dbus_atomic_inc (&connection->refcount); @@ -1031,7 +1219,9 @@ _dbus_connection_unref_unlocked (DBusConnection *connection) { dbus_bool_t last_unref; - _dbus_return_if_fail (connection != NULL); + HAVE_LOCK_CHECK (connection); + + _dbus_assert (connection != NULL); /* The connection lock is better than the global * lock in the atomic increment fallback @@ -1089,6 +1279,8 @@ _dbus_connection_handle_watch (DBusWatch *watch, DBusDispatchStatus status; connection = data; + + _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME); CONNECTION_LOCK (connection); _dbus_connection_acquire_io_path (connection, -1); @@ -1096,10 +1288,16 @@ _dbus_connection_handle_watch (DBusWatch *watch, watch, condition); _dbus_connection_release_io_path (connection); + HAVE_LOCK_CHECK (connection); + + _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); + status = _dbus_connection_get_dispatch_status_unlocked (connection); /* this calls out to user code */ _dbus_connection_update_dispatch_status_and_unlock (connection, status); + + _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME); return retval; } @@ -1154,7 +1352,10 @@ dbus_connection_open (const char *address, dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); return NULL; } - + +#ifndef DBUS_DISABLE_CHECKS + _dbus_assert (!connection->have_connection_lock); +#endif return connection; } @@ -1357,7 +1558,8 @@ dbus_connection_disconnect (DBusConnection *connection) CONNECTION_LOCK (connection); _dbus_transport_disconnect (connection->transport); - + + _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); status = _dbus_connection_get_dispatch_status_unlocked (connection); /* this calls out to user code */ @@ -1367,6 +1569,7 @@ dbus_connection_disconnect (DBusConnection *connection) static dbus_bool_t _dbus_connection_get_is_connected_unlocked (DBusConnection *connection) { + HAVE_LOCK_CHECK (connection); return _dbus_transport_get_is_connected (connection->transport); } @@ -1445,6 +1648,8 @@ _dbus_connection_preallocate_send_unlocked (DBusConnection *connection) { DBusPreallocatedSend *preallocated; + HAVE_LOCK_CHECK (connection); + _dbus_assert (connection != NULL); preallocated = dbus_new (DBusPreallocatedSend, 1); @@ -1604,9 +1809,9 @@ _dbus_connection_send_preallocated_unlocked_no_update (DBusConnection *con /* Now we need to run an iteration to hopefully just write the messages * out immediately, and otherwise get them queued up */ - _dbus_connection_do_iteration (connection, - DBUS_ITERATION_DO_WRITING, - -1); + _dbus_connection_do_iteration_unlocked (connection, + DBUS_ITERATION_DO_WRITING, + -1); /* If stuff is still queued up, be sure we wake up the main loop */ if (connection->n_outgoing > 0) @@ -1621,10 +1826,13 @@ _dbus_connection_send_preallocated_and_unlock (DBusConnection *connection, { DBusDispatchStatus status; + HAVE_LOCK_CHECK (connection); + _dbus_connection_send_preallocated_unlocked_no_update (connection, preallocated, message, client_serial); + _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); status = _dbus_connection_get_dispatch_status_unlocked (connection); /* this calls out to user code */ @@ -1699,7 +1907,10 @@ _dbus_connection_send_and_unlock (DBusConnection *connection, preallocated = _dbus_connection_preallocate_send_unlocked (connection); if (preallocated == NULL) - return FALSE; + { + CONNECTION_UNLOCK (connection); + return FALSE; + } _dbus_connection_send_preallocated_and_unlock (connection, preallocated, @@ -1762,6 +1973,7 @@ reply_handler_timeout (void *data) pending->timeout); pending->timeout_added = FALSE; + _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); status = _dbus_connection_get_dispatch_status_unlocked (connection); /* Unlocks, and calls out to user code */ @@ -1877,8 +2089,12 @@ dbus_connection_send_with_reply (DBusConnection *connection, if (pending_return) *pending_return = pending; else - dbus_pending_call_unref (pending); + { + _dbus_connection_detach_pending_call_unlocked (connection, pending); + dbus_pending_call_unref (pending); + } + _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); status = _dbus_connection_get_dispatch_status_unlocked (connection); /* this calls out to user code */ @@ -1898,6 +2114,8 @@ check_for_reply_unlocked (DBusConnection *connection, dbus_uint32_t client_serial) { DBusList *link; + + HAVE_LOCK_CHECK (connection); link = _dbus_list_get_first_link (&connection->incoming_messages); @@ -1980,14 +2198,19 @@ _dbus_connection_block_for_reply (DBusConnection *connection, * gets the message before we do? */ /* always block at least once as we know we don't have the reply yet */ - _dbus_connection_do_iteration (connection, - DBUS_ITERATION_DO_READING | - DBUS_ITERATION_BLOCK, - timeout_milliseconds); + _dbus_connection_do_iteration_unlocked (connection, + DBUS_ITERATION_DO_READING | + DBUS_ITERATION_BLOCK, + timeout_milliseconds); recheck_status: + _dbus_verbose ("%s top of recheck\n", _DBUS_FUNCTION_NAME); + + HAVE_LOCK_CHECK (connection); + /* queue messages and get status */ + status = _dbus_connection_get_dispatch_status_unlocked (connection); if (status == DBUS_DISPATCH_DATA_REMAINS) @@ -1996,7 +2219,8 @@ _dbus_connection_block_for_reply (DBusConnection *connection, reply = check_for_reply_unlocked (connection, client_serial); if (reply != NULL) - { + { + _dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME); status = _dbus_connection_get_dispatch_status_unlocked (connection); _dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n"); @@ -2011,7 +2235,10 @@ _dbus_connection_block_for_reply (DBusConnection *connection, _dbus_get_current_time (&tv_sec, &tv_usec); if (!_dbus_connection_get_is_connected_unlocked (connection)) - return NULL; + { + CONNECTION_UNLOCK (connection); + return NULL; + } else if (tv_sec < start_tv_sec) _dbus_verbose ("dbus_connection_send_with_reply_and_block(): clock set backward\n"); else if (connection->disconnect_message_link == NULL) @@ -2042,10 +2269,10 @@ _dbus_connection_block_for_reply (DBusConnection *connection, else { /* block again, we don't have the reply buffered yet. */ - _dbus_connection_do_iteration (connection, - DBUS_ITERATION_DO_READING | - DBUS_ITERATION_BLOCK, - timeout_milliseconds); + _dbus_connection_do_iteration_unlocked (connection, + DBUS_ITERATION_DO_READING | + DBUS_ITERATION_BLOCK, + timeout_milliseconds); } goto recheck_status; @@ -2144,16 +2371,25 @@ dbus_connection_flush (DBusConnection *connection) CONNECTION_LOCK (connection); while (connection->n_outgoing > 0 && _dbus_connection_get_is_connected_unlocked (connection)) - _dbus_connection_do_iteration (connection, - DBUS_ITERATION_DO_READING | - DBUS_ITERATION_DO_WRITING | - DBUS_ITERATION_BLOCK, - -1); + { + _dbus_verbose ("doing iteration in %s\n", _DBUS_FUNCTION_NAME); + HAVE_LOCK_CHECK (connection); + _dbus_connection_do_iteration_unlocked (connection, + DBUS_ITERATION_DO_READING | + DBUS_ITERATION_DO_WRITING | + DBUS_ITERATION_BLOCK, + -1); + } + HAVE_LOCK_CHECK (connection); + _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); status = _dbus_connection_get_dispatch_status_unlocked (connection); + HAVE_LOCK_CHECK (connection); /* Unlocks and calls out to user code */ _dbus_connection_update_dispatch_status_and_unlock (connection, status); + + _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME); } /* Call with mutex held. Will drop it while waiting and re-acquire @@ -2165,7 +2401,15 @@ _dbus_connection_wait_for_borrowed (DBusConnection *connection) _dbus_assert (connection->message_borrowed != NULL); while (connection->message_borrowed != NULL) - dbus_condvar_wait (connection->message_returned_cond, connection->mutex); + { +#ifndef DBUS_DISABLE_CHECKS + connection->have_connection_lock = FALSE; +#endif + dbus_condvar_wait (connection->message_returned_cond, connection->mutex); +#ifndef DBUS_DISABLE_CHECKS + connection->have_connection_lock = TRUE; +#endif + } } /** @@ -2191,6 +2435,8 @@ dbus_connection_borrow_message (DBusConnection *connection) _dbus_return_val_if_fail (connection != NULL, NULL); /* can't borrow during dispatch */ _dbus_return_val_if_fail (!connection->dispatch_acquired, NULL); + + _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME); /* this is called for the side effect that it queues * up any messages from the transport @@ -2283,6 +2529,8 @@ dbus_connection_steal_borrowed_message (DBusConnection *connection, static DBusList* _dbus_connection_pop_message_link_unlocked (DBusConnection *connection) { + HAVE_LOCK_CHECK (connection); + if (connection->message_borrowed != NULL) _dbus_connection_wait_for_borrowed (connection); @@ -2319,6 +2567,8 @@ static DBusMessage* _dbus_connection_pop_message_unlocked (DBusConnection *connection) { DBusList *link; + + HAVE_LOCK_CHECK (connection); link = _dbus_connection_pop_message_link_unlocked (connection); @@ -2340,6 +2590,8 @@ static void _dbus_connection_putback_message_link_unlocked (DBusConnection *connection, DBusList *message_link) { + HAVE_LOCK_CHECK (connection); + _dbus_assert (message_link != NULL); /* You can't borrow a message while a link is outstanding */ _dbus_assert (connection->message_borrowed == NULL); @@ -2381,6 +2633,8 @@ dbus_connection_pop_message (DBusConnection *connection) DBusMessage *message; DBusDispatchStatus status; + _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME); + /* this is called for the side effect that it queues * up any messages from the transport */ @@ -2411,7 +2665,15 @@ static void _dbus_connection_acquire_dispatch (DBusConnection *connection) { if (connection->dispatch_acquired) - dbus_condvar_wait (connection->dispatch_cond, connection->mutex); + { +#ifndef DBUS_DISABLE_CHECKS + connection->have_connection_lock = FALSE; +#endif + dbus_condvar_wait (connection->dispatch_cond, connection->mutex); +#ifndef DBUS_DISABLE_CHECKS + connection->have_connection_lock = TRUE; +#endif + } _dbus_assert (!connection->dispatch_acquired); connection->dispatch_acquired = TRUE; @@ -2445,6 +2707,8 @@ _dbus_connection_failed_pop (DBusConnection *connection, static DBusDispatchStatus _dbus_connection_get_dispatch_status_unlocked (DBusConnection *connection) { + HAVE_LOCK_CHECK (connection); + if (connection->n_incoming > 0) return DBUS_DISPATCH_DATA_REMAINS; else if (!_dbus_transport_queue_messages (connection->transport)) @@ -2511,7 +2775,7 @@ _dbus_connection_update_dispatch_status_and_unlock (DBusConnection *connectio DBusDispatchStatusFunction function; void *data; - /* We have the lock */ + HAVE_LOCK_CHECK (connection); _dbus_connection_ref_unlocked (connection); @@ -2550,6 +2814,8 @@ dbus_connection_get_dispatch_status (DBusConnection *connection) DBusDispatchStatus status; _dbus_return_val_if_fail (connection != NULL, DBUS_DISPATCH_COMPLETE); + + _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME); CONNECTION_LOCK (connection); @@ -2759,7 +3025,8 @@ dbus_connection_dispatch (DBusConnection *connection) dbus_message_get_member (message) : "no member", dbus_message_get_signature (message)); - + + HAVE_LOCK_CHECK (connection); result = _dbus_object_tree_dispatch_and_unlock (connection->objects, message); @@ -2874,7 +3141,8 @@ dbus_connection_dispatch (DBusConnection *connection) } _dbus_connection_release_dispatch (connection); - + + _dbus_verbose ("%s before final status update\n", _DBUS_FUNCTION_NAME); status = _dbus_connection_get_dispatch_status_unlocked (connection); /* unlocks and calls user code */ @@ -2952,25 +3220,43 @@ dbus_connection_set_watch_functions (DBusConnection *connection, DBusFreeFunction free_data_function) { dbus_bool_t retval; + DBusWatchList *watches; _dbus_return_val_if_fail (connection != NULL, FALSE); CONNECTION_LOCK (connection); + +#ifndef DBUS_DISABLE_CHECKS + if (connection->watches == NULL) + { + _dbus_warn ("Re-entrant call to %s is not allowed\n", + _DBUS_FUNCTION_NAME); + return FALSE; + } +#endif + /* ref connection for slightly better reentrancy */ _dbus_connection_ref_unlocked (connection); - /* FIXME this can call back into user code, and we need to drop the - * connection lock when it does. + /* This can call back into user code, and we need to drop the + * connection lock when it does. This is kind of a lame + * way to do it. */ - retval = _dbus_watch_list_set_functions (connection->watches, + watches = connection->watches; + connection->watches = NULL; + CONNECTION_UNLOCK (connection); + + retval = _dbus_watch_list_set_functions (watches, add_function, remove_function, toggled_function, data, free_data_function); + CONNECTION_LOCK (connection); + connection->watches = watches; CONNECTION_UNLOCK (connection); /* drop our paranoid refcount */ dbus_connection_unref (connection); - + return retval; } @@ -3016,17 +3302,34 @@ dbus_connection_set_timeout_functions (DBusConnection *connection, DBusFreeFunction free_data_function) { dbus_bool_t retval; + DBusTimeoutList *timeouts; _dbus_return_val_if_fail (connection != NULL, FALSE); CONNECTION_LOCK (connection); + +#ifndef DBUS_DISABLE_CHECKS + if (connection->timeouts == NULL) + { + _dbus_warn ("Re-entrant call to %s is not allowed\n", + _DBUS_FUNCTION_NAME); + return FALSE; + } +#endif + /* ref connection for slightly better reentrancy */ _dbus_connection_ref_unlocked (connection); + + timeouts = connection->timeouts; + connection->timeouts = NULL; + CONNECTION_UNLOCK (connection); - retval = _dbus_timeout_list_set_functions (connection->timeouts, + retval = _dbus_timeout_list_set_functions (timeouts, add_function, remove_function, toggled_function, data, free_data_function); + CONNECTION_LOCK (connection); + connection->timeouts = timeouts; CONNECTION_UNLOCK (connection); /* drop our paranoid refcount */ diff --git a/dbus/dbus-object-tree.c b/dbus/dbus-object-tree.c index ae28cc1c..b693b919 100644 --- a/dbus/dbus-object-tree.c +++ b/dbus/dbus-object-tree.c @@ -1,7 +1,7 @@ /* -*- mode: C; c-file-style: "gnu" -*- */ /* dbus-object-tree.c DBusObjectTree (internals of DBusConnection) * - * Copyright (C) 2003 Red Hat Inc. + * Copyright (C) 2003, 2005 Red Hat Inc. * * Licensed under the Academic Free License version 2.1 * @@ -503,6 +503,7 @@ _dbus_object_tree_unregister_and_unlock (DBusObjectTree *tree, #endif { _dbus_connection_ref_unlocked (connection); + _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME); _dbus_connection_unlock (connection); } @@ -635,7 +636,10 @@ handle_default_introspect_and_unlock (DBusObjectTree *tree, #ifdef DBUS_BUILD_TESTS if (tree->connection) #endif - _dbus_connection_unlock (tree->connection); + { + _dbus_verbose ("unlock %s %d\n", _DBUS_FUNCTION_NAME, __LINE__); + _dbus_connection_unlock (tree->connection); + } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; } @@ -647,7 +651,10 @@ handle_default_introspect_and_unlock (DBusObjectTree *tree, #ifdef DBUS_BUILD_TESTS if (tree->connection) #endif - _dbus_connection_unlock (tree->connection); + { + _dbus_verbose ("unlock %s %d\n", _DBUS_FUNCTION_NAME, __LINE__); + _dbus_connection_unlock (tree->connection); + } return DBUS_HANDLER_RESULT_NEED_MEMORY; } @@ -690,6 +697,8 @@ handle_default_introspect_and_unlock (DBusObjectTree *tree, if (tree->connection) #endif { + already_unlocked = TRUE; + if (!_dbus_connection_send_and_unlock (tree->connection, reply, NULL)) goto out; } @@ -702,7 +711,10 @@ handle_default_introspect_and_unlock (DBusObjectTree *tree, #endif { if (!already_unlocked) - _dbus_connection_unlock (tree->connection); + { + _dbus_verbose ("unlock %s %d\n", _DBUS_FUNCTION_NAME, __LINE__); + _dbus_connection_unlock (tree->connection); + } } _dbus_string_free (&xml); @@ -747,7 +759,10 @@ _dbus_object_tree_dispatch_and_unlock (DBusObjectTree *tree, #ifdef DBUS_BUILD_TESTS if (tree->connection) #endif - _dbus_connection_unlock (tree->connection); + { + _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME); + _dbus_connection_unlock (tree->connection); + } _dbus_verbose ("No memory to get decomposed path\n"); @@ -759,7 +774,10 @@ _dbus_object_tree_dispatch_and_unlock (DBusObjectTree *tree, #ifdef DBUS_BUILD_TESTS if (tree->connection) #endif - _dbus_connection_unlock (tree->connection); + { + _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME); + _dbus_connection_unlock (tree->connection); + } _dbus_verbose ("No path field in message\n"); return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; @@ -822,7 +840,10 @@ _dbus_object_tree_dispatch_and_unlock (DBusObjectTree *tree, #ifdef DBUS_BUILD_TESTS if (tree->connection) #endif - _dbus_connection_unlock (tree->connection); + { + _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME); + _dbus_connection_unlock (tree->connection); + } /* FIXME you could unregister the subtree in another thread * before we invoke the callback, and I can't figure out a @@ -859,7 +880,10 @@ _dbus_object_tree_dispatch_and_unlock (DBusObjectTree *tree, #ifdef DBUS_BUILD_TESTS if (tree->connection) #endif - _dbus_connection_unlock (tree->connection); + { + _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME); + _dbus_connection_unlock (tree->connection); + } } while (list != NULL) @@ -993,7 +1017,10 @@ _dbus_object_tree_list_registered_and_unlock (DBusObjectTree *tree, #ifdef DBUS_BUILD_TESTS if (tree->connection) #endif - _dbus_connection_unlock (tree->connection); + { + _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME); + _dbus_connection_unlock (tree->connection); + } return result; } diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h index bce29ca8..c8aa8601 100644 --- a/dbus/dbus-server-protected.h +++ b/dbus/dbus-server-protected.h @@ -23,6 +23,7 @@ #ifndef DBUS_SERVER_PROTECTED_H #define DBUS_SERVER_PROTECTED_H +#include <config.h> #include <dbus/dbus-internals.h> #include <dbus/dbus-server.h> #include <dbus/dbus-timeout.h> @@ -51,8 +52,9 @@ struct DBusServerVTable */ struct DBusServer { - int refcount; /**< Reference count. */ + DBusAtomic refcount; /**< Reference count. */ const DBusServerVTable *vtable; /**< Virtual methods for this instance. */ + DBusMutex *mutex; /**< Lock on the server object */ DBusWatchList *watches; /**< Our watches */ DBusTimeoutList *timeouts; /**< Our timeouts */ @@ -74,6 +76,10 @@ struct DBusServer char **auth_mechanisms; /**< Array of allowed authentication mechanisms */ unsigned int disconnected : 1; /**< TRUE if we are disconnected. */ + +#ifndef DBUS_DISABLE_CHECKS + unsigned int have_server_lock : 1; /**< Does someone have the server mutex locked */ +#endif }; dbus_bool_t _dbus_server_init_base (DBusServer *server, @@ -95,7 +101,38 @@ void _dbus_server_toggle_timeout (DBusServer *server, DBusTimeout *timeout, dbus_bool_t enabled); +void _dbus_server_ref_unlocked (DBusServer *server); + +#ifdef DBUS_DISABLE_CHECKS +#define TOOK_LOCK_CHECK(server) +#define RELEASING_LOCK_CHECK(server) +#define HAVE_LOCK_CHECK(server) +#else +#define TOOK_LOCK_CHECK(server) do { \ + _dbus_assert (!(server)->have_server_lock); \ + (server)->have_server_lock = TRUE; \ + } while (0) +#define RELEASING_LOCK_CHECK(server) do { \ + _dbus_assert ((server)->have_server_lock); \ + (server)->have_server_lock = FALSE; \ + } while (0) +#define HAVE_LOCK_CHECK(server) _dbus_assert ((server)->have_server_lock) +/* A "DO_NOT_HAVE_LOCK_CHECK" is impossible since we need the lock to check the flag */ +#endif + +#define TRACE_LOCKS 0 + +#define SERVER_LOCK(server) do { \ + if (TRACE_LOCKS) { _dbus_verbose (" LOCK: %s\n", _DBUS_FUNCTION_NAME); } \ + 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); \ + } while (0) DBUS_END_DECLS diff --git a/dbus/dbus-server-unix.c b/dbus/dbus-server-unix.c index f576b427..4a07f149 100644 --- a/dbus/dbus-server-unix.c +++ b/dbus/dbus-server-unix.c @@ -72,21 +72,29 @@ unix_finalize (DBusServer *server) */ /* Return value is just for memory, not other failures. */ static dbus_bool_t -handle_new_client_fd (DBusServer *server, - int client_fd) +handle_new_client_fd_and_unlock (DBusServer *server, + int client_fd) { DBusConnection *connection; DBusTransport *transport; + DBusNewConnectionFunction new_connection_function; + void *new_connection_data; _dbus_verbose ("Creating new client connection with fd %d\n", client_fd); - + + HAVE_LOCK_CHECK (server); + if (!_dbus_set_fd_nonblocking (client_fd, NULL)) - return TRUE; + { + SERVER_UNLOCK (server); + return TRUE; + } transport = _dbus_transport_new_for_fd (client_fd, TRUE, NULL); if (transport == NULL) { close (client_fd); + SERVER_UNLOCK (server); return FALSE; } @@ -94,6 +102,7 @@ handle_new_client_fd (DBusServer *server, (const char **) server->auth_mechanisms)) { _dbus_transport_unref (transport); + SERVER_UNLOCK (server); return FALSE; } @@ -103,19 +112,27 @@ handle_new_client_fd (DBusServer *server, connection = _dbus_connection_new_for_transport (transport); _dbus_transport_unref (transport); + transport = NULL; /* now under the connection lock */ if (connection == NULL) - return FALSE; + { + SERVER_UNLOCK (server); + return FALSE; + } - /* See if someone wants to handle this new connection, - * self-referencing for paranoia + /* See if someone wants to handle this new connection, self-referencing + * for paranoia. */ - if (server->new_connection_function) + new_connection_function = server->new_connection_function; + new_connection_data = server->new_connection_data; + + _dbus_server_ref_unlocked (server); + SERVER_UNLOCK (server); + + if (new_connection_function) { - dbus_server_ref (server); - - (* server->new_connection_function) (server, connection, - server->new_connection_data); + (* new_connection_function) (server, connection, + new_connection_data); dbus_server_unref (server); } @@ -133,6 +150,8 @@ unix_handle_watch (DBusWatch *watch, DBusServer *server = data; DBusServerUnix *unix_server = data; + SERVER_LOCK (server); + _dbus_assert (watch == unix_server->watch); _dbus_verbose ("Handling client connection, flags 0x%x\n", flags); @@ -155,12 +174,14 @@ unix_handle_watch (DBusWatch *watch, else _dbus_verbose ("Failed to accept a client connection: %s\n", _dbus_strerror (errno)); + + SERVER_UNLOCK (server); } else { _dbus_fd_set_close_on_exec (client_fd); - if (!handle_new_client_fd (server, client_fd)) + if (!handle_new_client_fd_and_unlock (server, client_fd)) _dbus_verbose ("Rejected client connection due to lack of memory\n"); } } @@ -246,6 +267,10 @@ _dbus_server_new_for_fd (int fd, return NULL; } +#ifndef DBUS_DISABLE_CHECKS + unix_server->base.have_server_lock = TRUE; +#endif + if (!_dbus_server_add_watch (&unix_server->base, watch)) { @@ -254,6 +279,10 @@ _dbus_server_new_for_fd (int fd, 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; diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c index 4c706191..788aaad7 100644 --- a/dbus/dbus-server.c +++ b/dbus/dbus-server.c @@ -67,7 +67,7 @@ _dbus_server_init_base (DBusServer *server, const DBusString *address) { server->vtable = vtable; - server->refcount = 1; + server->refcount.value = 1; server->address = NULL; server->watches = NULL; @@ -75,6 +75,10 @@ _dbus_server_init_base (DBusServer *server, if (!_dbus_string_copy_data (address, &server->address)) goto failed; + + server->mutex = dbus_mutex_new (); + if (server->mutex == NULL) + goto failed; server->watches = _dbus_watch_list_new (); if (server->watches == NULL) @@ -91,6 +95,11 @@ _dbus_server_init_base (DBusServer *server, return TRUE; failed: + if (server->mutex) + { + dbus_mutex_free (server->mutex); + server->mutex = NULL; + } if (server->watches) { _dbus_watch_list_free (server->watches); @@ -118,7 +127,7 @@ _dbus_server_init_base (DBusServer *server, */ void _dbus_server_finalize_base (DBusServer *server) -{ +{ /* calls out to application code... */ _dbus_data_slot_list_free (&server->slot_list); @@ -130,6 +139,8 @@ _dbus_server_finalize_base (DBusServer *server) _dbus_watch_list_free (server->watches); _dbus_timeout_list_free (server->timeouts); + dbus_mutex_free (server->mutex); + dbus_free (server->address); dbus_free_string_array (server->auth_mechanisms); @@ -146,6 +157,7 @@ dbus_bool_t _dbus_server_add_watch (DBusServer *server, DBusWatch *watch) { + HAVE_LOCK_CHECK (server); return _dbus_watch_list_add_watch (server->watches, watch); } @@ -159,6 +171,7 @@ void _dbus_server_remove_watch (DBusServer *server, DBusWatch *watch) { + HAVE_LOCK_CHECK (server); _dbus_watch_list_remove_watch (server->watches, watch); } @@ -176,6 +189,8 @@ _dbus_server_toggle_watch (DBusServer *server, DBusWatch *watch, dbus_bool_t enabled) { + HAVE_LOCK_CHECK (server); + if (server->watches) /* null during finalize */ _dbus_watch_list_toggle_watch (server->watches, watch, enabled); @@ -194,6 +209,8 @@ dbus_bool_t _dbus_server_add_timeout (DBusServer *server, DBusTimeout *timeout) { + HAVE_LOCK_CHECK (server); + return _dbus_timeout_list_add_timeout (server->timeouts, timeout); } @@ -207,6 +224,8 @@ void _dbus_server_remove_timeout (DBusServer *server, DBusTimeout *timeout) { + HAVE_LOCK_CHECK (server); + _dbus_timeout_list_remove_timeout (server->timeouts, timeout); } @@ -224,6 +243,8 @@ _dbus_server_toggle_timeout (DBusServer *server, DBusTimeout *timeout, dbus_bool_t enabled) { + HAVE_LOCK_CHECK (server); + if (server->timeouts) /* null during finalize */ _dbus_timeout_list_toggle_timeout (server->timeouts, timeout, enabled); @@ -457,8 +478,16 @@ DBusServer * dbus_server_ref (DBusServer *server) { _dbus_return_val_if_fail (server != NULL, NULL); - - server->refcount += 1; + +#ifdef DBUS_HAVE_ATOMIC_INT + _dbus_atomic_inc (&server->refcount); +#else + SERVER_LOCK (server); + _dbus_assert (server->refcount.value > 0); + + server->refcount.value += 1; + SERVER_UNLOCK (server); +#endif return server; } @@ -474,12 +503,24 @@ dbus_server_ref (DBusServer *server) void dbus_server_unref (DBusServer *server) { + dbus_bool_t last_unref; + _dbus_return_if_fail (server != NULL); - _dbus_assert (server->refcount > 0); +#ifdef DBUS_HAVE_ATOMIC_INT + last_unref = (_dbus_atomic_dec (&server->refcount) == 1); +#else + SERVER_LOCK (server); + + _dbus_assert (server->refcount.value > 0); - server->refcount -= 1; - if (server->refcount == 0) + server->refcount.value -= 1; + last_unref = (server->refcount.value == 0); + + SERVER_UNLOCK (server); +#endif + + if (last_unref) { _dbus_assert (server->vtable->finalize != NULL); @@ -488,6 +529,25 @@ dbus_server_unref (DBusServer *server) } /** + * Like dbus_server_ref() but does not acquire the lock (must already be held) + * + * @param server the server. + */ +void +_dbus_server_ref_unlocked (DBusServer *server) +{ + HAVE_LOCK_CHECK (server); + +#ifdef DBUS_HAVE_ATOMIC_INT + _dbus_atomic_inc (&server->refcount); +#else + _dbus_assert (server->refcount.value > 0); + + server->refcount.value += 1; +#endif +} + +/** * Releases the server's address and stops listening for * new clients. If called more than once, only the first * call has an effect. Does not modify the server's @@ -499,6 +559,8 @@ void dbus_server_disconnect (DBusServer *server) { _dbus_return_if_fail (server != NULL); + + SERVER_LOCK (server); _dbus_assert (server->vtable->disconnect != NULL); @@ -507,6 +569,8 @@ dbus_server_disconnect (DBusServer *server) (* server->vtable->disconnect) (server); server->disconnected = TRUE; + + SERVER_UNLOCK (server); } /** @@ -517,9 +581,15 @@ dbus_server_disconnect (DBusServer *server) dbus_bool_t dbus_server_get_is_connected (DBusServer *server) { - _dbus_return_val_if_fail (server != NULL, FALSE); + dbus_bool_t retval; - return !server->disconnected; + _dbus_return_val_if_fail (server != NULL, FALSE); + + SERVER_LOCK (server); + retval = !server->disconnected; + SERVER_UNLOCK (server); + + return retval; } /** @@ -532,9 +602,15 @@ dbus_server_get_is_connected (DBusServer *server) char* dbus_server_get_address (DBusServer *server) { - _dbus_return_val_if_fail (server != NULL, NULL); + char *retval; - return _dbus_strdup (server->address); + _dbus_return_val_if_fail (server != NULL, NULL); + + SERVER_LOCK (server); + retval = _dbus_strdup (server->address); + SERVER_UNLOCK (server); + + return retval; } /** @@ -555,14 +631,22 @@ dbus_server_set_new_connection_function (DBusServer *server, void *data, DBusFreeFunction free_data_function) { - _dbus_return_if_fail (server != NULL); + DBusFreeFunction old_free_function; + void *old_data; - if (server->new_connection_free_data_function != NULL) - (* server->new_connection_free_data_function) (server->new_connection_data); + _dbus_return_if_fail (server != NULL); + + SERVER_LOCK (server); + old_free_function = server->new_connection_free_data_function; + old_data = server->new_connection_data; server->new_connection_function = function; server->new_connection_data = data; server->new_connection_free_data_function = free_data_function; + SERVER_UNLOCK (server); + + if (old_free_function != NULL) + (* old_free_function) (old_data); } /** @@ -589,14 +673,34 @@ dbus_server_set_watch_functions (DBusServer *server, void *data, DBusFreeFunction free_data_function) { + dbus_bool_t result; + DBusWatchList *watches; + _dbus_return_val_if_fail (server != NULL, FALSE); + + SERVER_LOCK (server); + watches = server->watches; + server->watches = NULL; + if (watches) + { + SERVER_UNLOCK (server); + result = _dbus_watch_list_set_functions (watches, + add_function, + remove_function, + toggled_function, + data, + free_data_function); + SERVER_LOCK (server); + } + else + { + _dbus_warn ("Re-entrant call to %s\n", _DBUS_FUNCTION_NAME); + result = FALSE; + } + server->watches = watches; + SERVER_UNLOCK (server); - return _dbus_watch_list_set_functions (server->watches, - add_function, - remove_function, - toggled_function, - data, - free_data_function); + return result; } /** @@ -622,12 +726,34 @@ dbus_server_set_timeout_functions (DBusServer *server, void *data, DBusFreeFunction free_data_function) { + dbus_bool_t result; + DBusTimeoutList *timeouts; + _dbus_return_val_if_fail (server != NULL, FALSE); + + SERVER_LOCK (server); + timeouts = server->timeouts; + server->timeouts = NULL; + if (timeouts) + { + SERVER_UNLOCK (server); + result = _dbus_timeout_list_set_functions (timeouts, + add_function, + remove_function, + toggled_function, + data, + free_data_function); + SERVER_LOCK (server); + } + else + { + _dbus_warn ("Re-entrant call to %s\n", _DBUS_FUNCTION_NAME); + result = FALSE; + } + server->timeouts = timeouts; + SERVER_UNLOCK (server); - return _dbus_timeout_list_set_functions (server->timeouts, - add_function, remove_function, - toggled_function, - data, free_data_function); + return result; } /** @@ -647,6 +773,8 @@ dbus_server_set_auth_mechanisms (DBusServer *server, char **copy; _dbus_return_val_if_fail (server != NULL, FALSE); + + SERVER_LOCK (server); if (mechanisms != NULL) { @@ -660,6 +788,8 @@ dbus_server_set_auth_mechanisms (DBusServer *server, dbus_free_string_array (server->auth_mechanisms); server->auth_mechanisms = copy; + SERVER_UNLOCK (server); + return TRUE; } @@ -732,19 +862,16 @@ dbus_server_set_data (DBusServer *server, dbus_bool_t retval; _dbus_return_val_if_fail (server != NULL, FALSE); - -#if 0 - dbus_mutex_lock (server->mutex); -#endif + + SERVER_LOCK (server); retval = _dbus_data_slot_list_set (&slot_allocator, &server->slot_list, slot, data, free_data_func, &old_free_func, &old_data); -#if 0 - dbus_mutex_unlock (server->mutex); -#endif + + SERVER_UNLOCK (server); if (retval) { @@ -772,17 +899,13 @@ dbus_server_get_data (DBusServer *server, _dbus_return_val_if_fail (server != NULL, NULL); -#if 0 - dbus_mutex_lock (server->mutex); -#endif + SERVER_LOCK (server); res = _dbus_data_slot_list_get (&slot_allocator, &server->slot_list, slot); -#if 0 - dbus_mutex_unlock (server->mutex); -#endif + SERVER_UNLOCK (server); return res; } diff --git a/dbus/dbus-transport-unix.c b/dbus/dbus-transport-unix.c index 4190da46..2959886a 100644 --- a/dbus/dbus-transport-unix.c +++ b/dbus/dbus-transport-unix.c @@ -70,6 +70,8 @@ static void free_watches (DBusTransport *transport) { DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; + + _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME); if (unix_transport->read_watch) { @@ -90,12 +92,16 @@ free_watches (DBusTransport *transport) _dbus_watch_unref (unix_transport->write_watch); unix_transport->write_watch = NULL; } + + _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME); } static void unix_finalize (DBusTransport *transport) { DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; + + _dbus_verbose ("%s\n", _DBUS_FUNCTION_NAME); free_watches (transport); @@ -871,6 +877,8 @@ static void unix_disconnect (DBusTransport *transport) { DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport; + + _dbus_verbose ("%s\n", _DBUS_FUNCTION_NAME); free_watches (transport); @@ -1004,7 +1012,10 @@ unix_do_iteration (DBusTransport *transport, * by the io_path_cond condvar, so we won't reenter this. */ if (flags & DBUS_ITERATION_BLOCK) - _dbus_connection_unlock (transport->connection); + { + _dbus_verbose ("unlock %s pre poll\n", _DBUS_FUNCTION_NAME); + _dbus_connection_unlock (transport->connection); + } again: poll_res = _dbus_poll (&poll_fd, 1, poll_timeout); @@ -1013,7 +1024,10 @@ unix_do_iteration (DBusTransport *transport, goto again; if (flags & DBUS_ITERATION_BLOCK) - _dbus_connection_lock (transport->connection); + { + _dbus_verbose ("lock %s post poll\n", _DBUS_FUNCTION_NAME); + _dbus_connection_lock (transport->connection); + } if (poll_res >= 0) { diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index 94d4a8a8..c08573d8 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -388,10 +388,12 @@ _dbus_transport_unref (DBusTransport *transport) { _dbus_assert (transport != NULL); _dbus_assert (transport->refcount > 0); - + transport->refcount -= 1; if (transport->refcount == 0) { + _dbus_verbose ("%s: finalizing\n", _DBUS_FUNCTION_NAME); + _dbus_assert (transport->vtable->finalize != NULL); (* transport->vtable->finalize) (transport); @@ -409,14 +411,18 @@ _dbus_transport_unref (DBusTransport *transport) void _dbus_transport_disconnect (DBusTransport *transport) { + _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME); + _dbus_assert (transport->vtable->disconnect != NULL); - + if (transport->disconnected) return; (* transport->vtable->disconnect) (transport); transport->disconnected = TRUE; + + _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME); } /** @@ -437,7 +443,8 @@ _dbus_transport_get_is_connected (DBusTransport *transport) * Returns #TRUE if we have been authenticated. Will return #TRUE * even if the transport is disconnected. * - * @todo needs to drop connection->mutex when calling the unix_user_function + * @todo we drop connection->mutex when calling the unix_user_function, + * which may not be safe really. * * @param transport the transport * @returns whether we're authenticated @@ -453,6 +460,9 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport) if (transport->disconnected) return FALSE; + + /* paranoia ref since we call user callbacks sometimes */ + _dbus_connection_ref_unlocked (transport->connection); maybe_authenticated = (!(transport->send_credentials_pending || @@ -486,21 +496,40 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport) if (transport->unix_user_function != NULL) { - /* FIXME we hold the connection lock here and should drop it */ - if (!(* transport->unix_user_function) (transport->connection, - auth_identity.uid, - transport->unix_user_data)) + dbus_bool_t allow; + DBusConnection *connection; + DBusAllowUnixUserFunction unix_user_function; + void *unix_user_data; + + /* Dropping the lock here probably isn't that safe. */ + + connection = transport->connection; + unix_user_function = transport->unix_user_function; + unix_user_data = transport->unix_user_data; + + _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME); + _dbus_connection_unlock (connection); + + allow = (* unix_user_function) (connection, + auth_identity.uid, + unix_user_data); + + _dbus_verbose ("lock %s post unix user function\n", _DBUS_FUNCTION_NAME); + _dbus_connection_lock (connection); + + if (allow) + { + _dbus_verbose ("Client UID "DBUS_UID_FORMAT" authorized\n", auth_identity.uid); + } + else { _dbus_verbose ("Client UID "DBUS_UID_FORMAT " was rejected, disconnecting\n", auth_identity.uid); _dbus_transport_disconnect (transport); + _dbus_connection_unref_unlocked (connection); return FALSE; } - else - { - _dbus_verbose ("Client UID "DBUS_UID_FORMAT" authorized\n", auth_identity.uid); - } } else { @@ -515,6 +544,7 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport) " but our UID is "DBUS_UID_FORMAT", disconnecting\n", auth_identity.uid, our_identity.uid); _dbus_transport_disconnect (transport); + _dbus_connection_unref_unlocked (transport->connection); return FALSE; } else @@ -527,8 +557,9 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport) } transport->authenticated = maybe_authenticated; - - return transport->authenticated; + + _dbus_connection_unref_unlocked (transport->connection); + return maybe_authenticated; } } @@ -670,6 +701,8 @@ _dbus_transport_do_iteration (DBusTransport *transport, (* transport->vtable->do_iteration) (transport, flags, timeout_milliseconds); _dbus_transport_unref (transport); + + _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME); } static dbus_bool_t |