From: Alexander Larsson Date: Sat, 15 Feb 2003 16:25:08 +0000 (+0000) Subject: 2003-02-15 Alexander Larsson X-Git-Tag: dbus-0.4~24 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=fe4018941190f8bf020e4a8ed2999c212e0e113d;p=platform%2Fupstream%2Fdbus.git 2003-02-15 Alexander Larsson * dbus/dbus-threads.c: * dbus/dbus-threads.h: Add condvars. Remove static mutext from API. Implement static mutexes by initializing them from threads_init. * glib/dbus-gthread.c: * qt/dbus-qthread.cpp: Update with the thread api changes. * dbus/dbus-list.c: * dbus/dbus-list.h: Turn StaticMutex into normal mutex + init function. Export new functions _dbus_list_alloc_link, _dbus_list_free_link, _dbus_list_append_link, _dbus_list_prepend_link * dbus/dbus-sysdeps.c: * dbus/dbus-sysdeps.h: New type dbus_atomic_t, and new functions _dbus_atomic_inc, _dbus_atomic_dec. Only slow fallback implementation at the moment. * dbus/dbus-protocol.h: Add DBUS_MESSAGE_LOCAL_DISCONNECT define * dbus/dbus-message.c: Make ref/unref atomic. Fix some docs. * dbus/dbus-connection-internal.h: * dbus/dbus-connection.c: * dbus/dbus-connection.h: Make threadsafe. Change _peek to _borrow,_return & _steal_borrowed. Change disconnect callback to event. Make dbus_connection_dispatch_messages reentrant. * dbus/dbus-transport.c: Don't ref the connection on calls to the transport implementation. * dbus/dbus-message-handler.c: Make threadsafe. * glib/dbus-gmain.c: Don't use peek_message anymore * test/Makefile.am: * test/debug-thread.c: * test/debug-thread.h: Simple thread implementation that asserts() on deadlocks in single-threaded code. * test/bus-test.c: (main) Call debug_threads_init. * test/watch.c: Use disconnect message instead of disconnect callback. * bus/connection.c: * bus/connection.h: Don't call dbus_connection_set_disconnect_function. Instead export bus_connection_disconnect. * bus/dispatch.c: Call bus_connection_disconnect when we get a disconnected message. --- diff --git a/ChangeLog b/ChangeLog index bce8784..77b8000 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,72 @@ +2003-02-15 Alexander Larsson + + * dbus/dbus-threads.c: + * dbus/dbus-threads.h: + Add condvars. Remove static mutext from API. + Implement static mutexes by initializing them from threads_init. + + * glib/dbus-gthread.c: + * qt/dbus-qthread.cpp: + Update with the thread api changes. + + + * dbus/dbus-list.c: + * dbus/dbus-list.h: + Turn StaticMutex into normal mutex + init function. + Export new functions _dbus_list_alloc_link, _dbus_list_free_link, + _dbus_list_append_link, _dbus_list_prepend_link + + + * dbus/dbus-sysdeps.c: + * dbus/dbus-sysdeps.h: + New type dbus_atomic_t, and new functions _dbus_atomic_inc, + _dbus_atomic_dec. Only slow fallback implementation at the moment. + + * dbus/dbus-protocol.h: + Add DBUS_MESSAGE_LOCAL_DISCONNECT define + + * dbus/dbus-message.c: + Make ref/unref atomic. + Fix some docs. + + * dbus/dbus-connection-internal.h: + * dbus/dbus-connection.c: + * dbus/dbus-connection.h: + Make threadsafe. + Change _peek to _borrow,_return & _steal_borrowed. + Change disconnect callback to event. + Make dbus_connection_dispatch_messages reentrant. + + * dbus/dbus-transport.c: + Don't ref the connection on calls to the transport + implementation. + + * dbus/dbus-message-handler.c: + Make threadsafe. + + * glib/dbus-gmain.c: + Don't use peek_message anymore + + * test/Makefile.am: + * test/debug-thread.c: + * test/debug-thread.h: + Simple thread implementation that asserts() on deadlocks in + single-threaded code. + + * test/bus-test.c: + (main) Call debug_threads_init. + + * test/watch.c: + Use disconnect message instead of disconnect callback. + + * bus/connection.c: + * bus/connection.h: + Don't call dbus_connection_set_disconnect_function. Instead export + bus_connection_disconnect. + + * bus/dispatch.c: + Call bus_connection_disconnect when we get a disconnected message. + 2003-02-15 Havoc Pennington * dbus/dbus-message.c (dbus_message_new): fool around with the diff --git a/bus/connection.c b/bus/connection.c index 2119ed9..40bbc32 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -38,9 +38,8 @@ typedef struct #define BUS_CONNECTION_DATA(connection) (dbus_connection_get_data ((connection), connection_data_slot)) -static void -connection_disconnect_handler (DBusConnection *connection, - void *data) +void +bus_connection_disconnect (DBusConnection *connection) { BusConnectionData *d; BusService *service; @@ -157,10 +156,6 @@ bus_connection_setup (DBusConnection *connection) connection, NULL); - dbus_connection_set_disconnect_function (connection, - connection_disconnect_handler, - NULL, NULL); - /* Setup the connection with the dispatcher */ if (!bus_dispatch_add_connection (connection)) return FALSE; diff --git a/bus/connection.h b/bus/connection.h index 8a6b664..04ab1f0 100644 --- a/bus/connection.h +++ b/bus/connection.h @@ -47,5 +47,8 @@ const char *bus_connection_get_name (DBusConnection *connection); void bus_connection_foreach (BusConnectionForeachFunction function, void *data); +/* called by dispatch.c */ +void bus_connection_disconnect (DBusConnection *connection); + #endif /* BUS_CONNECTION_H */ diff --git a/bus/dispatch.c b/bus/dispatch.c index 5b06959..4e42f9a 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -51,16 +51,23 @@ bus_dispatch_message_handler (DBusMessageHandler *handler, DBusMessage *message, void *user_data) { - const char *sender, *service_name; + const char *sender, *service_name, *message_name; /* Assign a sender to the message */ sender = bus_connection_get_name (connection); BUS_HANDLE_OOM (dbus_message_set_sender (message, sender)); service_name = dbus_message_get_service (message); + message_name = dbus_message_get_name (message); + + /* TODO: Crashes if service_name == NULL */ /* See if the message is to the driver */ - if (strcmp (service_name, DBUS_SERVICE_DBUS) == 0) + if (message_name && strcmp (message_name, DBUS_MESSAGE_LOCAL_DISCONNECT) == 0) + { + bus_connection_disconnect (connection); + } + else if (strcmp (service_name, DBUS_SERVICE_DBUS) == 0) { bus_driver_handle_message (connection, message); } diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h index ae94961..0606d1b 100644 --- a/dbus/dbus-connection-internal.h +++ b/dbus/dbus-connection-internal.h @@ -38,6 +38,8 @@ typedef enum DBUS_ITERATION_BLOCK = 1 << 2 /**< Block if nothing to do. */ } DBusIterationFlags; +void _dbus_connection_ref_unlocked (DBusConnection *connection); + dbus_bool_t _dbus_connection_queue_received_message (DBusConnection *connection, DBusMessage *message); dbus_bool_t _dbus_connection_have_messages_to_send (DBusConnection *connection); @@ -60,14 +62,14 @@ void _dbus_connection_do_iteration (DBusConnection *connect unsigned int flags, int timeout_milliseconds); -void _dbus_connection_notify_disconnected (DBusConnection *connection); +void _dbus_connection_notify_disconnected (DBusConnection *connection); -void _dbus_connection_handler_destroyed (DBusConnection *connection, - DBusMessageHandler *handler); +void _dbus_connection_handler_destroyed_locked (DBusConnection *connection, + DBusMessageHandler *handler); -void _dbus_connection_set_connection_counter (DBusConnection *connection, - DBusCounter *counter); +void _dbus_connection_set_connection_counter (DBusConnection *connection, + DBusCounter *counter); dbus_bool_t _dbus_message_handler_add_connection (DBusMessageHandler *handler, DBusConnection *connection); diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 3bc8d2d..73b9f18 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -30,7 +30,9 @@ #include "dbus-list.h" #include "dbus-hash.h" #include "dbus-message-internal.h" +#include "dbus-message-handler.h" #include "dbus-threads.h" +#include "dbus-protocol.h" /** * @defgroup DBusConnection DBusConnection @@ -78,9 +80,22 @@ struct DBusConnection { int refcount; /**< Reference count. */ + DBusMutex *mutex; + + /* Protects dispatch_message */ + dbus_bool_t dispatch_acquired; + DBusCondVar *dispatch_cond; + + /* Protects transport io path */ + dbus_bool_t io_path_acquired; + DBusCondVar *io_path_cond; + DBusList *outgoing_messages; /**< Queue of messages we need to send, send the end of the list first. */ DBusList *incoming_messages; /**< Queue of messages we have received, end of the list received most recently. */ + DBusMessage *message_borrowed; /**< True if the first incoming message has been borrowed */ + DBusCondVar *message_returned_cond; + int n_outgoing; /**< Length of outgoing queue. */ int n_incoming; /**< Length of incoming queue. */ @@ -88,23 +103,18 @@ struct DBusConnection DBusWatchList *watches; /**< Stores active watches. */ DBusTimeoutList *timeouts; /**< Stores active timeouts. */ - DBusDisconnectFunction disconnect_function; /**< Callback on disconnect. */ - void *disconnect_data; /**< Data for disconnect callback. */ - DBusFreeFunction disconnect_free_data_function; /**< Free function for disconnect callback data. */ DBusHashTable *handler_table; /**< Table of registered DBusMessageHandler */ DBusList *filter_list; /**< List of filters. */ - int filters_serial; /**< Increments when the list of filters is changed. */ - int handlers_serial; /**< Increments when the handler table is changed. */ DBusDataSlot *data_slots; /**< Data slots */ int n_slots; /**< Slots allocated so far. */ DBusCounter *connection_counter; /**< Counter that we decrement when finalized */ int client_serial; /**< Client serial. Increments each time a message is sent */ - unsigned int disconnect_notified : 1; /**< Already called disconnect_function */ + DBusList *disconnect_message_link; }; -static void _dbus_connection_free_data_slots (DBusConnection *connection); +static void _dbus_connection_free_data_slots_nolock (DBusConnection *connection); /** * Adds a message to the incoming message queue, returning #FALSE @@ -134,6 +144,29 @@ _dbus_connection_queue_received_message (DBusConnection *connection, } /** + * Adds a link + message to the incoming message queue. + * Can't fail. Takes ownership of both link and message. + * + * @param connection the connection. + * @param link the list node and message to queue. + * + * @todo This needs to wake up the mainloop if it is in + * a poll/select and this is a multithreaded app. + */ +static void +_dbus_connection_queue_synthesized_message_link (DBusConnection *connection, + DBusList *link) +{ + _dbus_list_append_link (&connection->incoming_messages, link); + + connection->n_incoming += 1; + + _dbus_verbose ("Incoming synthesized message %p added to queue, %d incoming\n", + link->data, connection->n_incoming); +} + + +/** * Checks whether there are messages in the outgoing message queue. * * @param connection the connection. @@ -263,25 +296,72 @@ _dbus_connection_remove_timeout (DBusConnection *connection, /** * Tells the connection that the transport has been disconnected. - * Results in calling the application disconnect callback. - * Only has an effect the first time it's called. + * Results in posting a disconnect message on the incoming message + * queue. Only has an effect the first time it's called. * * @param connection the connection */ void _dbus_connection_notify_disconnected (DBusConnection *connection) { - if (connection->disconnect_function != NULL && - !connection->disconnect_notified) + if (connection->disconnect_message_link) { - connection->disconnect_notified = TRUE; - dbus_connection_ref (connection); - (* connection->disconnect_function) (connection, - connection->disconnect_data); - dbus_connection_unref (connection); + /* We haven't sent the disconnect message already */ + _dbus_connection_queue_synthesized_message_link (connection, + connection->disconnect_message_link); + connection->disconnect_message_link = NULL; } } + +/** + * Acquire the transporter I/O path. This must be done before + * doing any I/O in the transporter. May sleep and drop the + * connection mutex while waiting for the I/O path. + * + * @param connection the connection. + * @param timeout_milliseconds maximum blocking time, or -1 for no limit. + * @returns TRUE if the I/O path was acquired. + */ +static dbus_bool_t +_dbus_connection_acquire_io_path (DBusConnection *connection, + int timeout_milliseconds) +{ + dbus_bool_t res = TRUE; + 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); + + if (res) + { + _dbus_assert (!connection->io_path_acquired); + + connection->io_path_acquired = TRUE; + } + + return res; +} + +/** + * Release the I/O path when you're done with it. Only call + * after you've acquired the I/O. Wakes up at most one thread + * currently waiting to acquire the I/O path. + * + * @param connection the connection. + */ +static void +_dbus_connection_release_io_path (DBusConnection *connection) +{ + _dbus_assert (connection->io_path_acquired); + + connection->io_path_acquired = FALSE; + dbus_condvar_wake_one (connection->io_path_cond); +} + + /** * Queues incoming messages and sends outgoing messages for this * connection, optionally blocking in the process. Each call to @@ -315,9 +395,14 @@ _dbus_connection_do_iteration (DBusConnection *connection, { if (connection->n_outgoing == 0) flags &= ~DBUS_ITERATION_DO_WRITING; - - _dbus_transport_do_iteration (connection->transport, - flags, timeout_milliseconds); + + if (_dbus_connection_acquire_io_path (connection, + (flags & DBUS_ITERATION_BLOCK)?timeout_milliseconds:0)) + { + _dbus_transport_do_iteration (connection->transport, + flags, timeout_milliseconds); + _dbus_connection_release_io_path (connection); + } } /** @@ -336,11 +421,23 @@ _dbus_connection_new_for_transport (DBusTransport *transport) DBusWatchList *watch_list; DBusTimeoutList *timeout_list; DBusHashTable *handler_table; + DBusMutex *mutex; + DBusCondVar *message_returned_cond; + DBusCondVar *dispatch_cond; + DBusCondVar *io_path_cond; + DBusList *disconnect_link; + DBusMessage *disconnect_message; watch_list = NULL; connection = NULL; handler_table = NULL; timeout_list = NULL; + mutex = NULL; + message_returned_cond = NULL; + dispatch_cond = NULL; + io_path_cond = NULL; + disconnect_link = NULL; + disconnect_message = NULL; watch_list = _dbus_watch_list_new (); if (watch_list == NULL) @@ -359,8 +456,36 @@ _dbus_connection_new_for_transport (DBusTransport *transport) connection = dbus_new0 (DBusConnection, 1); if (connection == NULL) goto error; + + mutex = dbus_mutex_new (); + if (mutex == NULL) + goto error; + + message_returned_cond = dbus_condvar_new (); + if (message_returned_cond == NULL) + goto error; + + dispatch_cond = dbus_condvar_new (); + if (dispatch_cond == NULL) + goto error; + + io_path_cond = dbus_condvar_new (); + if (io_path_cond == NULL) + goto error; + + disconnect_message = dbus_message_new (NULL, DBUS_MESSAGE_LOCAL_DISCONNECT); + if (disconnect_message == NULL) + goto error; + + disconnect_link = _dbus_list_alloc_link (disconnect_message); + if (disconnect_link == NULL) + goto error; connection->refcount = 1; + connection->mutex = mutex; + connection->dispatch_cond = dispatch_cond; + connection->io_path_cond = io_path_cond; + connection->message_returned_cond = message_returned_cond; connection->transport = transport; connection->watches = watch_list; connection->timeouts = timeout_list; @@ -370,7 +495,8 @@ _dbus_connection_new_for_transport (DBusTransport *transport) connection->data_slots = NULL; connection->n_slots = 0; connection->client_serial = 1; - connection->disconnect_notified = FALSE; + + connection->disconnect_message_link = disconnect_link; _dbus_transport_ref (transport); _dbus_transport_set_connection (transport, connection); @@ -378,6 +504,23 @@ _dbus_connection_new_for_transport (DBusTransport *transport) return connection; error: + if (disconnect_message != NULL) + dbus_message_unref (disconnect_message); + + if (disconnect_link != NULL) + _dbus_list_free_link (disconnect_link); + + if (io_path_cond != NULL) + dbus_condvar_free (io_path_cond); + + if (dispatch_cond != NULL) + dbus_condvar_free (dispatch_cond); + + if (message_returned_cond != NULL) + dbus_condvar_free (message_returned_cond); + + if (mutex != NULL) + dbus_mutex_free (mutex); if (connection != NULL) dbus_free (connection); @@ -410,18 +553,21 @@ _dbus_connection_get_next_client_serial (DBusConnection *connection) /** * Used to notify a connection when a DBusMessageHandler is * destroyed, so the connection can drop any reference - * to the handler. + * to the handler. This is a private function, but still + * takes the connection lock. Don't call it with the lock held. * * @param connection the connection * @param handler the handler */ void -_dbus_connection_handler_destroyed (DBusConnection *connection, - DBusMessageHandler *handler) +_dbus_connection_handler_destroyed_locked (DBusConnection *connection, + DBusMessageHandler *handler) { DBusHashIter iter; DBusList *link; + dbus_mutex_lock (connection->mutex); + _dbus_hash_iter_init (connection->handler_table, &iter); while (_dbus_hash_iter_next (&iter)) { @@ -443,6 +589,7 @@ _dbus_connection_handler_destroyed (DBusConnection *connection, link = next; } + dbus_mutex_unlock (connection->mutex); } /** @@ -520,89 +667,130 @@ dbus_connection_open (const char *address, void dbus_connection_ref (DBusConnection *connection) { + dbus_mutex_lock (connection->mutex); + _dbus_assert (connection->refcount > 0); + connection->refcount += 1; + dbus_mutex_unlock (connection->mutex); } /** - * Decrements the reference count of a DBusConnection, and finalizes - * it if the count reaches zero. If a connection is still connected - * when it's finalized, it will be disconnected (that is, associated - * file handles will be closed). + * Increments the reference count of a DBusConnection. + * Requires that the caller already holds the connection lock. * * @param connection the connection. */ void -dbus_connection_unref (DBusConnection *connection) +_dbus_connection_ref_unlocked (DBusConnection *connection) { - _dbus_assert (connection != NULL); _dbus_assert (connection->refcount > 0); + connection->refcount += 1; +} - connection->refcount -= 1; - if (connection->refcount == 0) - { - DBusHashIter iter; - DBusList *link; - - dbus_connection_disconnect (connection); - - /* free disconnect data as a side effect */ - dbus_connection_set_disconnect_function (connection, - NULL, NULL, NULL); - if (connection->connection_counter != NULL) - { - /* subtract ourselves from the counter */ - _dbus_counter_adjust (connection->connection_counter, - 1); - _dbus_counter_unref (connection->connection_counter); - connection->connection_counter = NULL; - } +/* This is run without the mutex held, but after the last reference + to the connection has been dropped we should have no thread-related + problems */ +static void +_dbus_connection_last_unref (DBusConnection *connection) +{ + DBusHashIter iter; + DBusList *link; - _dbus_watch_list_free (connection->watches); - connection->watches = NULL; + _dbus_assert (!_dbus_transport_get_is_connected (connection->transport)); + + if (connection->connection_counter != NULL) + { + /* subtract ourselves from the counter */ + _dbus_counter_adjust (connection->connection_counter, - 1); + _dbus_counter_unref (connection->connection_counter); + connection->connection_counter = NULL; + } + + _dbus_watch_list_free (connection->watches); + connection->watches = NULL; + + _dbus_timeout_list_free (connection->timeouts); + connection->timeouts = NULL; + + _dbus_connection_free_data_slots_nolock (connection); + + _dbus_hash_iter_init (connection->handler_table, &iter); + while (_dbus_hash_iter_next (&iter)) + { + DBusMessageHandler *h = _dbus_hash_iter_get_value (&iter); - _dbus_timeout_list_free (connection->timeouts); - connection->timeouts = NULL; - - _dbus_connection_free_data_slots (connection); + _dbus_message_handler_remove_connection (h, connection); + } + + link = _dbus_list_get_first_link (&connection->filter_list); + while (link != NULL) + { + DBusMessageHandler *h = link->data; + DBusList *next = _dbus_list_get_next_link (&connection->filter_list, link); - _dbus_hash_iter_init (connection->handler_table, &iter); - while (_dbus_hash_iter_next (&iter)) - { - DBusMessageHandler *h = _dbus_hash_iter_get_value (&iter); - - _dbus_message_handler_remove_connection (h, connection); - } - - link = _dbus_list_get_first_link (&connection->filter_list); - while (link != NULL) - { - DBusMessageHandler *h = link->data; - DBusList *next = _dbus_list_get_next_link (&connection->filter_list, link); - - _dbus_message_handler_remove_connection (h, connection); - - link = next; - } + _dbus_message_handler_remove_connection (h, connection); - _dbus_hash_table_unref (connection->handler_table); - connection->handler_table = NULL; + link = next; + } + + _dbus_hash_table_unref (connection->handler_table); + connection->handler_table = NULL; + + _dbus_list_clear (&connection->filter_list); + + _dbus_list_foreach (&connection->outgoing_messages, + (DBusForeachFunction) dbus_message_unref, + NULL); + _dbus_list_clear (&connection->outgoing_messages); + + _dbus_list_foreach (&connection->incoming_messages, + (DBusForeachFunction) dbus_message_unref, + NULL); + _dbus_list_clear (&connection->incoming_messages); + + _dbus_transport_unref (connection->transport); - _dbus_list_clear (&connection->filter_list); - - _dbus_list_foreach (&connection->outgoing_messages, - (DBusForeachFunction) dbus_message_unref, - NULL); - _dbus_list_clear (&connection->outgoing_messages); - - _dbus_list_foreach (&connection->incoming_messages, - (DBusForeachFunction) dbus_message_unref, - NULL); - _dbus_list_clear (&connection->incoming_messages); - - _dbus_transport_unref (connection->transport); - - dbus_free (connection); + if (connection->disconnect_message_link) + { + DBusMessage *message = connection->disconnect_message_link->data; + dbus_message_unref (message); + _dbus_list_free_link (connection->disconnect_message_link); } + + dbus_condvar_free (connection->dispatch_cond); + dbus_condvar_free (connection->io_path_cond); + dbus_condvar_free (connection->message_returned_cond); + + dbus_mutex_free (connection->mutex); + + dbus_free (connection); +} + +/** + * Decrements the reference count of a DBusConnection, and finalizes + * it if the count reaches zero. It is a bug to drop the last reference + * to a connection that has not been disconnected. + * + * @param connection the connection. + */ +void +dbus_connection_unref (DBusConnection *connection) +{ + dbus_bool_t last_unref; + + dbus_mutex_lock (connection->mutex); + + _dbus_assert (connection != NULL); + _dbus_assert (connection->refcount > 0); + + connection->refcount -= 1; + last_unref = (connection->refcount == 0); + + dbus_mutex_unlock (connection->mutex); + + if (last_unref) + _dbus_connection_last_unref (connection); } /** @@ -618,7 +806,9 @@ dbus_connection_unref (DBusConnection *connection) void dbus_connection_disconnect (DBusConnection *connection) { + dbus_mutex_lock (connection->mutex); _dbus_transport_disconnect (connection->transport); + dbus_mutex_unlock (connection->mutex); } /** @@ -634,7 +824,13 @@ dbus_connection_disconnect (DBusConnection *connection) dbus_bool_t dbus_connection_get_is_connected (DBusConnection *connection) { - return _dbus_transport_get_is_connected (connection->transport); + dbus_bool_t res; + + dbus_mutex_lock (connection->mutex); + res = _dbus_transport_get_is_connected (connection->transport); + dbus_mutex_unlock (connection->mutex); + + return res; } /** @@ -648,7 +844,13 @@ dbus_connection_get_is_connected (DBusConnection *connection) dbus_bool_t dbus_connection_get_is_authenticated (DBusConnection *connection) { - return _dbus_transport_get_is_authenticated (connection->transport); + dbus_bool_t res; + + dbus_mutex_lock (connection->mutex); + res = _dbus_transport_get_is_authenticated (connection->transport); + dbus_mutex_unlock (connection->mutex); + + return res; } /** @@ -675,11 +877,14 @@ dbus_connection_send_message (DBusConnection *connection, { dbus_int32_t serial; - + + dbus_mutex_lock (connection->mutex); + if (!_dbus_list_prepend (&connection->outgoing_messages, message)) { dbus_set_result (result, DBUS_RESULT_NO_MEMORY); + dbus_mutex_unlock (connection->mutex); return FALSE; } @@ -699,11 +904,13 @@ dbus_connection_send_message (DBusConnection *connection, *client_serial = _dbus_message_get_client_serial (message); _dbus_message_lock (message); - + if (connection->n_outgoing == 1) _dbus_transport_messages_pending (connection->transport, connection->n_outgoing); + dbus_mutex_unlock (connection->mutex); + return TRUE; } @@ -800,8 +1007,13 @@ dbus_connection_send_message_with_reply_and_block (DBusConnection *connectio /* Flush message queue */ dbus_connection_flush (connection); + + dbus_mutex_lock (connection->mutex); /* Now we wait... */ + /* THREAD TODO: This is busted. What if a dispatch_message or pop_message + * gets the message before we do? + */ _dbus_connection_do_iteration (connection, DBUS_ITERATION_DO_READING | DBUS_ITERATION_BLOCK, @@ -822,6 +1034,7 @@ dbus_connection_send_message_with_reply_and_block (DBusConnection *connectio if (result) *result = DBUS_RESULT_SUCCESS; + dbus_mutex_unlock (connection->mutex); return reply; } link = _dbus_list_get_next_link (&connection->incoming_messages, link); @@ -829,7 +1042,9 @@ dbus_connection_send_message_with_reply_and_block (DBusConnection *connectio if (result) *result = DBUS_RESULT_NO_REPLY; - + + dbus_mutex_unlock (connection->mutex); + return NULL; } @@ -841,11 +1056,13 @@ dbus_connection_send_message_with_reply_and_block (DBusConnection *connectio void dbus_connection_flush (DBusConnection *connection) { + dbus_mutex_lock (connection->mutex); while (connection->n_outgoing > 0) _dbus_connection_do_iteration (connection, DBUS_ITERATION_DO_WRITING | DBUS_ITERATION_BLOCK, -1); + dbus_mutex_unlock (connection->mutex); } /** @@ -857,34 +1074,112 @@ dbus_connection_flush (DBusConnection *connection) int dbus_connection_get_n_messages (DBusConnection *connection) { - return connection->n_incoming; + int res; + + dbus_mutex_lock (connection->mutex); + res = connection->n_incoming; + dbus_mutex_unlock (connection->mutex); + return res; +} + + +/* Call with mutex held. Will drop it while waiting and re-acquire + before returning */ +static void +_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); } /** * Returns the first-received message from the incoming message queue, - * leaving it in the queue. The caller does not own a reference to the - * returned message. If the queue is empty, returns #NULL. + * leaving it in the queue. If the queue is empty, returns #NULL. + * + * The caller does not own a reference to the returned message, and must + * either return it using dbus_connection_return_message or keep it after + * calling dbus_connection_steal_borrowed_message. No one can get at the + * message while its borrowed, so return it as quickly as possible and + * don't keep a reference to it after returning it. If you need to keep + * the message, make a copy of it. * * @param connection the connection. * @returns next message in the incoming queue. */ DBusMessage* -dbus_connection_peek_message (DBusConnection *connection) +dbus_connection_borrow_message (DBusConnection *connection) { - return _dbus_list_get_first (&connection->incoming_messages); + DBusMessage *message; + + dbus_mutex_lock (connection->mutex); + + if (connection->message_borrowed != NULL) + _dbus_connection_wait_for_borrowed (connection); + + message = _dbus_list_get_first (&connection->incoming_messages); + + if (message) + connection->message_borrowed = message; + + dbus_mutex_unlock (connection->mutex); + return message; } /** - * Returns the first-received message from the incoming message queue, - * removing it from the queue. The caller owns a reference to the - * returned message. If the queue is empty, returns #NULL. - * - * @param connection the connection. - * @returns next message in the incoming queue. + * @todo docs */ -DBusMessage* -dbus_connection_pop_message (DBusConnection *connection) +void +dbus_connection_return_message (DBusConnection *connection, + DBusMessage *message) +{ + dbus_mutex_lock (connection->mutex); + + _dbus_assert (message == connection->message_borrowed); + + connection->message_borrowed = NULL; + dbus_condvar_wake_all (connection->message_returned_cond); + + dbus_mutex_unlock (connection->mutex); +} + +/** + * @todo docs + */ +void +dbus_connection_steal_borrowed_message (DBusConnection *connection, + DBusMessage *message) { + DBusMessage *pop_message; + + dbus_mutex_lock (connection->mutex); + + _dbus_assert (message == connection->message_borrowed); + + pop_message = _dbus_list_pop_first (&connection->incoming_messages); + _dbus_assert (message == pop_message); + + connection->n_incoming -= 1; + + _dbus_verbose ("Incoming message %p stolen from queue, %d incoming\n", + message, connection->n_incoming); + + connection->message_borrowed = NULL; + dbus_condvar_wake_all (connection->message_returned_cond); + + dbus_mutex_unlock (connection->mutex); +} + + +/* See dbus_connection_pop_message, but requires the caller to own + the lock before calling. May drop the lock while running. */ +static DBusMessage* +_dbus_connection_pop_message_unlocked (DBusConnection *connection) +{ + if (connection->message_borrowed != NULL) + _dbus_connection_wait_for_borrowed (connection); + if (connection->n_incoming > 0) { DBusMessage *message; @@ -901,62 +1196,141 @@ dbus_connection_pop_message (DBusConnection *connection) return NULL; } + +/** + * Returns the first-received message from the incoming message queue, + * removing it from the queue. The caller owns a reference to the + * returned message. If the queue is empty, returns #NULL. + * + * @param connection the connection. + * @returns next message in the incoming queue. + */ +DBusMessage* +dbus_connection_pop_message (DBusConnection *connection) +{ + DBusMessage *message; + dbus_mutex_lock (connection->mutex); + + message = _dbus_connection_pop_message_unlocked (connection); + + dbus_mutex_unlock (connection->mutex); + + return message; +} + +/** + * Acquire the dispatcher. This must be done before dispatching + * messages in order to guarantee the right order of + * message delivery. May sleep and drop the connection mutex + * while waiting for the dispatcher. + * + * @param connection the connection. + */ +static void +_dbus_connection_acquire_dispatch (DBusConnection *connection) +{ + dbus_condvar_wait (connection->dispatch_cond, connection->mutex); + _dbus_assert (!connection->dispatch_acquired); + + connection->dispatch_acquired = TRUE; +} + +/** + * Release the dispatcher when you're done with it. Only call + * after you've acquired the dispatcher. Wakes up at most one + * thread currently waiting to acquire the dispatcher. + * + * @param connection the connection. + */ +static void +_dbus_connection_release_dispatch (DBusConnection *connection) +{ + _dbus_assert (connection->dispatch_acquired); + + connection->dispatch_acquired = FALSE; + dbus_condvar_wake_one (connection->dispatch_cond); +} + /** * Pops the first-received message from the current incoming message * queue, runs any handlers for it, then unrefs the message. * * @param connection the connection * @returns #TRUE if the queue is not empty after dispatch - * - * @todo this function is not properly robust against reentrancy, - * that is, if handlers are added/removed while dispatching - * a message, things will get messed up. */ dbus_bool_t dbus_connection_dispatch_message (DBusConnection *connection) { DBusMessage *message; - int filter_serial; - int handler_serial; - DBusList *link; + DBusList *link, *filter_list_copy; DBusHandlerResult result; const char *name; + + dbus_mutex_lock (connection->mutex); + + /* We need to ref the connection since the callback could potentially + * drop the last ref to it */ + _dbus_connection_ref_unlocked (connection); + + _dbus_connection_acquire_dispatch (connection); - dbus_connection_ref (connection); - - message = dbus_connection_pop_message (connection); + /* This call may drop the lock during the execution (if waiting + for borrowed messages to be returned) but the order of message + dispatch if several threads call dispatch_message is still + protected by the lock, since only one will get the lock, and that + one will finish the message dispatching */ + message = _dbus_connection_pop_message_unlocked (connection); if (message == NULL) { + _dbus_connection_release_dispatch (connection); + dbus_mutex_unlock (connection->mutex); dbus_connection_unref (connection); return FALSE; } - filter_serial = connection->filters_serial; - handler_serial = connection->handlers_serial; - result = DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS; + + if (!_dbus_list_copy (&connection->filter_list, &filter_list_copy)) + { + _dbus_connection_release_dispatch (connection); + dbus_mutex_unlock (connection->mutex); + dbus_connection_unref (connection); + return FALSE; + } - link = _dbus_list_get_first_link (&connection->filter_list); + _dbus_list_foreach (&filter_list_copy, + (DBusForeachFunction)dbus_message_handler_ref, + NULL); + + /* We're still protected from dispatch_message reentrancy here + * since we acquired the dispatcher */ + dbus_mutex_unlock (connection->mutex); + + link = _dbus_list_get_first_link (&filter_list_copy); while (link != NULL) { DBusMessageHandler *handler = link->data; - DBusList *next = _dbus_list_get_next_link (&connection->filter_list, link); - + DBusList *next = _dbus_list_get_next_link (&filter_list_copy, link); + result = _dbus_message_handler_handle_message (handler, connection, message); if (result == DBUS_HANDLER_RESULT_REMOVE_MESSAGE) - goto out; + break; - if (filter_serial != connection->filters_serial) - { - _dbus_warn ("Message filters added or removed while dispatching filters - not currently supported!\n"); - goto out; - } - link = next; } + _dbus_list_foreach (&filter_list_copy, + (DBusForeachFunction)dbus_message_handler_unref, + NULL); + _dbus_list_clear (&filter_list_copy); + + dbus_mutex_lock (connection->mutex); + + if (result == DBUS_HANDLER_RESULT_REMOVE_MESSAGE) + goto out; + name = dbus_message_get_name (message); if (name != NULL) { @@ -966,22 +1340,20 @@ dbus_connection_dispatch_message (DBusConnection *connection) name); if (handler != NULL) { - + /* We're still protected from dispatch_message reentrancy here + * since we acquired the dispatcher */ + dbus_mutex_unlock (connection->mutex); result = _dbus_message_handler_handle_message (handler, connection, message); - + dbus_mutex_lock (connection->mutex); if (result == DBUS_HANDLER_RESULT_REMOVE_MESSAGE) goto out; - - if (handler_serial != connection->handlers_serial) - { - _dbus_warn ("Message handlers added or removed while dispatching handlers - not currently supported!\n"); - goto out; - } } } out: + _dbus_connection_release_dispatch (connection); + dbus_mutex_unlock (connection->mutex); dbus_connection_unref (connection); dbus_message_unref (message); @@ -989,30 +1361,6 @@ dbus_connection_dispatch_message (DBusConnection *connection) } /** - * Sets the disconnect handler function for the connection. - * Will be called exactly once, when the connection is - * disconnected. - * - * @param connection the connection. - * @param disconnect_function the disconnect handler. - * @param data data to pass to the disconnect handler. - * @param free_data_function function to be called to free the data. - */ -void -dbus_connection_set_disconnect_function (DBusConnection *connection, - DBusDisconnectFunction disconnect_function, - void *data, - DBusFreeFunction free_data_function) -{ - if (connection->disconnect_free_data_function != NULL) - (* connection->disconnect_free_data_function) (connection->disconnect_data); - - connection->disconnect_function = disconnect_function; - connection->disconnect_data = data; - connection->disconnect_free_data_function = free_data_function; -} - -/** * Sets the watch functions for the connection. These functions are * responsible for making the application's main loop aware of file * descriptors that need to be monitored for events, using select() or @@ -1053,13 +1401,15 @@ dbus_connection_set_watch_functions (DBusConnection *connection, void *data, DBusFreeFunction free_data_function) { + dbus_mutex_lock (connection->mutex); /* ref connection for slightly better reentrancy */ - dbus_connection_ref (connection); + _dbus_connection_ref_unlocked (connection); _dbus_watch_list_set_functions (connection->watches, add_function, remove_function, data, free_data_function); + dbus_mutex_unlock (connection->mutex); /* drop our paranoid refcount */ dbus_connection_unref (connection); } @@ -1090,13 +1440,15 @@ dbus_connection_set_timeout_functions (DBusConnection *connection, void *data, DBusFreeFunction free_data_function) { + dbus_mutex_lock (connection->mutex); /* ref connection for slightly better reentrancy */ - dbus_connection_ref (connection); + _dbus_connection_ref_unlocked (connection); _dbus_timeout_list_set_functions (connection->timeouts, add_function, remove_function, data, free_data_function); + dbus_mutex_unlock (connection->mutex); /* drop our paranoid refcount */ dbus_connection_unref (connection); } @@ -1115,8 +1467,12 @@ dbus_connection_handle_watch (DBusConnection *connection, DBusWatch *watch, unsigned int condition) { + dbus_mutex_lock (connection->mutex); + _dbus_connection_acquire_io_path (connection, -1); _dbus_transport_handle_watch (connection->transport, - watch, condition); + watch, condition); + _dbus_connection_release_io_path (connection); + dbus_mutex_unlock (connection->mutex); } /** @@ -1126,6 +1482,8 @@ dbus_connection_handle_watch (DBusConnection *connection, * Filters are run in the order that they were added. * The same handler can be added as a filter more than once, in * which case it will be run more than once. + * Filters added during a filter callback won't be run on the + * message being processed. * * @param connection the connection * @param handler the handler @@ -1135,18 +1493,22 @@ dbus_bool_t dbus_connection_add_filter (DBusConnection *connection, DBusMessageHandler *handler) { + dbus_mutex_lock (connection->mutex); if (!_dbus_message_handler_add_connection (handler, connection)) - return FALSE; + { + dbus_mutex_unlock (connection->mutex); + return FALSE; + } if (!_dbus_list_append (&connection->filter_list, handler)) { _dbus_message_handler_remove_connection (handler, connection); + dbus_mutex_unlock (connection->mutex); return FALSE; } - connection->filters_serial += 1; - + dbus_mutex_unlock (connection->mutex); return TRUE; } @@ -1165,15 +1527,17 @@ void dbus_connection_remove_filter (DBusConnection *connection, DBusMessageHandler *handler) { + dbus_mutex_lock (connection->mutex); if (!_dbus_list_remove_last (&connection->filter_list, handler)) { _dbus_warn ("Tried to remove a DBusConnection filter that had not been added\n"); + dbus_mutex_unlock (connection->mutex); return; } _dbus_message_handler_remove_connection (handler, connection); - connection->filters_serial += 1; + dbus_mutex_unlock (connection->mutex); } /** @@ -1199,6 +1563,7 @@ dbus_connection_register_handler (DBusConnection *connection, { int i; + dbus_mutex_lock (connection->mutex); i = 0; while (i < n_messages) { @@ -1234,11 +1599,10 @@ dbus_connection_register_handler (DBusConnection *connection, _dbus_hash_iter_set_value (&iter, handler); - connection->handlers_serial += 1; - ++i; } + dbus_mutex_unlock (connection->mutex); return TRUE; failed: @@ -1250,6 +1614,7 @@ dbus_connection_register_handler (DBusConnection *connection, messages_to_handle, i); + dbus_mutex_unlock (connection->mutex); return FALSE; } @@ -1271,6 +1636,7 @@ dbus_connection_unregister_handler (DBusConnection *connection, { int i; + dbus_mutex_lock (connection->mutex); i = 0; while (i < n_messages) { @@ -1297,13 +1663,22 @@ dbus_connection_unregister_handler (DBusConnection *connection, ++i; } - connection->handlers_serial += 1; + dbus_mutex_unlock (connection->mutex); } static int *allocated_slots = NULL; static int n_allocated_slots = 0; static int n_used_slots = 0; -static DBusStaticMutex allocated_slots_lock = DBUS_STATIC_MUTEX_INIT; +static DBusMutex *allocated_slots_lock = NULL; + +DBusMutex *_dbus_allocated_slots_init_lock (void); +DBusMutex * +_dbus_allocated_slots_init_lock (void) +{ + allocated_slots_lock = dbus_mutex_new (); + return allocated_slots_lock; +} + /** * Allocates an integer ID to be used for storing application-specific @@ -1318,7 +1693,7 @@ dbus_connection_allocate_data_slot (void) { int slot; - if (!dbus_static_mutex_lock (&allocated_slots_lock)) + if (!dbus_mutex_lock (allocated_slots_lock)) return -1; if (n_used_slots < n_allocated_slots) @@ -1358,7 +1733,7 @@ dbus_connection_allocate_data_slot (void) _dbus_assert (slot < n_allocated_slots); out: - dbus_static_mutex_unlock (&allocated_slots_lock); + dbus_mutex_unlock (allocated_slots_lock); return slot; } @@ -1376,7 +1751,7 @@ dbus_connection_allocate_data_slot (void) void dbus_connection_free_data_slot (int slot) { - dbus_static_mutex_lock (&allocated_slots_lock); + dbus_mutex_lock (allocated_slots_lock); _dbus_assert (slot < n_allocated_slots); _dbus_assert (allocated_slots[slot] == slot); @@ -1391,7 +1766,7 @@ dbus_connection_free_data_slot (int slot) n_allocated_slots = 0; } - dbus_static_mutex_unlock (&allocated_slots_lock); + dbus_mutex_unlock (allocated_slots_lock); } /** @@ -1413,6 +1788,10 @@ dbus_connection_set_data (DBusConnection *connection, void *data, DBusFreeFunction free_data_func) { + DBusFreeFunction old_free_func; + void *old_data; + + dbus_mutex_lock (connection->mutex); _dbus_assert (slot < n_allocated_slots); _dbus_assert (allocated_slots[slot] == slot); @@ -1424,7 +1803,10 @@ dbus_connection_set_data (DBusConnection *connection, tmp = dbus_realloc (connection->data_slots, sizeof (DBusDataSlot) * (slot + 1)); if (tmp == NULL) - return FALSE; + { + dbus_mutex_unlock (connection->mutex); + return FALSE; + } connection->data_slots = tmp; i = connection->n_slots; @@ -1438,13 +1820,19 @@ dbus_connection_set_data (DBusConnection *connection, } _dbus_assert (slot < connection->n_slots); - - if (connection->data_slots[slot].free_data_func) - (* connection->data_slots[slot].free_data_func) (connection->data_slots[slot].data); + + old_data = connection->data_slots[slot].data; + old_free_func = connection->data_slots[slot].free_data_func; connection->data_slots[slot].data = data; connection->data_slots[slot].free_data_func = free_data_func; + dbus_mutex_unlock (connection->mutex); + + /* Do the actual free outside the connection lock */ + if (old_free_func) + (* old_free_func) (old_data); + return TRUE; } @@ -1460,17 +1848,29 @@ void* dbus_connection_get_data (DBusConnection *connection, int slot) { + void *res; + + dbus_mutex_lock (connection->mutex); + _dbus_assert (slot < n_allocated_slots); _dbus_assert (allocated_slots[slot] == slot); if (slot >= connection->n_slots) - return NULL; + res = NULL; + else + res = connection->data_slots[slot].data; - return connection->data_slots[slot].data; + dbus_mutex_unlock (connection->mutex); + + return res; } +/* This must be called with the connection lock not held to avoid + * holding it over the free_data callbacks, so it can basically + * only be called at last unref + */ static void -_dbus_connection_free_data_slots (DBusConnection *connection) +_dbus_connection_free_data_slots_nolock (DBusConnection *connection) { int i; @@ -1501,8 +1901,10 @@ void dbus_connection_set_max_message_size (DBusConnection *connection, long size) { + dbus_mutex_lock (connection->mutex); _dbus_transport_set_max_message_size (connection->transport, size); + dbus_mutex_unlock (connection->mutex); } /** @@ -1514,7 +1916,11 @@ dbus_connection_set_max_message_size (DBusConnection *connection, long dbus_connection_get_max_message_size (DBusConnection *connection) { - return _dbus_transport_get_max_message_size (connection->transport); + long res; + dbus_mutex_lock (connection->mutex); + res = _dbus_transport_get_max_message_size (connection->transport); + dbus_mutex_unlock (connection->mutex); + return res; } /** @@ -1543,8 +1949,10 @@ void dbus_connection_set_max_live_messages_size (DBusConnection *connection, long size) { + dbus_mutex_lock (connection->mutex); _dbus_transport_set_max_live_messages_size (connection->transport, size); + dbus_mutex_unlock (connection->mutex); } /** @@ -1556,7 +1964,11 @@ dbus_connection_set_max_live_messages_size (DBusConnection *connection, long dbus_connection_get_max_live_messages_size (DBusConnection *connection) { - return _dbus_transport_get_max_live_messages_size (connection->transport); + long res; + dbus_mutex_lock (connection->mutex); + res = _dbus_transport_get_max_live_messages_size (connection->transport); + dbus_mutex_unlock (connection->mutex); + return res; } /** @} */ diff --git a/dbus/dbus-connection.h b/dbus/dbus-connection.h index 036d677..774374e 100644 --- a/dbus/dbus-connection.h +++ b/dbus/dbus-connection.h @@ -66,21 +66,23 @@ typedef void (* DBusAddTimeoutFunction) (DBusTimeout *timeout, typedef void (* DBusRemoveTimeoutFunction) (DBusTimeout *timeout, void *data); -typedef void (* DBusDisconnectFunction) (DBusConnection *connection, - void *data); +DBusConnection* dbus_connection_open (const char *address, + DBusResultCode *result); +void dbus_connection_ref (DBusConnection *connection); +void dbus_connection_unref (DBusConnection *connection); +void dbus_connection_disconnect (DBusConnection *connection); +dbus_bool_t dbus_connection_get_is_connected (DBusConnection *connection); +dbus_bool_t dbus_connection_get_is_authenticated (DBusConnection *connection); +void dbus_connection_flush (DBusConnection *connection); +int dbus_connection_get_n_messages (DBusConnection *connection); +DBusMessage* dbus_connection_borrow_message (DBusConnection *connection); +void dbus_connection_return_message (DBusConnection *connection, + DBusMessage *message); +void dbus_connection_steal_borrowed_message (DBusConnection *connection, + DBusMessage *message); +DBusMessage* dbus_connection_pop_message (DBusConnection *connection); +dbus_bool_t dbus_connection_dispatch_message (DBusConnection *connection); -DBusConnection* dbus_connection_open (const char *address, - DBusResultCode *result); -void dbus_connection_ref (DBusConnection *connection); -void dbus_connection_unref (DBusConnection *connection); -void dbus_connection_disconnect (DBusConnection *connection); -dbus_bool_t dbus_connection_get_is_connected (DBusConnection *connection); -dbus_bool_t dbus_connection_get_is_authenticated (DBusConnection *connection); -void dbus_connection_flush (DBusConnection *connection); -int dbus_connection_get_n_messages (DBusConnection *connection); -DBusMessage* dbus_connection_peek_message (DBusConnection *connection); -DBusMessage* dbus_connection_pop_message (DBusConnection *connection); -dbus_bool_t dbus_connection_dispatch_message (DBusConnection *connection); dbus_bool_t dbus_connection_send_message (DBusConnection *connection, DBusMessage *message, @@ -97,10 +99,6 @@ DBusMessage *dbus_connection_send_message_with_reply_and_block (DBusConnection DBusResultCode *result); -void dbus_connection_set_disconnect_function (DBusConnection *connection, - DBusDisconnectFunction function, - void *data, - DBusFreeFunction free_data_function); void dbus_connection_set_watch_functions (DBusConnection *connection, DBusAddWatchFunction add_function, DBusRemoveWatchFunction remove_function, diff --git a/dbus/dbus-list.c b/dbus/dbus-list.c index e71a7c4..7f12e3d 100644 --- a/dbus/dbus-list.c +++ b/dbus/dbus-list.c @@ -35,7 +35,15 @@ */ static DBusMemPool *list_pool; -static DBusStaticMutex list_pool_lock = DBUS_STATIC_MUTEX_INIT; +static DBusMutex *list_pool_lock = NULL; + +DBusMutex *_dbus_list_init_lock (void); +DBusMutex * +_dbus_list_init_lock (void) +{ + list_pool_lock = dbus_mutex_new (); + return list_pool_lock; +} /** * @defgroup DBusListInternals Linked list implementation details @@ -55,7 +63,7 @@ alloc_link (void *data) { DBusList *link; - if (!dbus_static_mutex_lock (&list_pool_lock)) + if (!dbus_mutex_lock (list_pool_lock)) return NULL; if (!list_pool) @@ -64,7 +72,7 @@ alloc_link (void *data) if (list_pool == NULL) { - dbus_static_mutex_unlock (&list_pool_lock); + dbus_mutex_unlock (list_pool_lock); return NULL; } } @@ -72,7 +80,7 @@ alloc_link (void *data) link = _dbus_mem_pool_alloc (list_pool); link->data = data; - dbus_static_mutex_unlock (&list_pool_lock); + dbus_mutex_unlock (list_pool_lock); return link; } @@ -80,9 +88,9 @@ alloc_link (void *data) static void free_link (DBusList *link) { - dbus_static_mutex_lock (&list_pool_lock); + dbus_mutex_lock (list_pool_lock); _dbus_mem_pool_dealloc (list_pool, link); - dbus_static_mutex_unlock (&list_pool_lock); + dbus_mutex_unlock (list_pool_lock); } static void @@ -190,6 +198,33 @@ link_after (DBusList **list, */ /** + * Allocates a linked list node. Useful for preallocating + * nodes and using _dbus_list_append_link() to avoid + * allocations. + * + * @param data the value to store in the link. + * @returns a newly allocated link. + */ +DBusList* +_dbus_list_alloc_link (void *data) +{ + return alloc_link (data); +} + +/** + * Frees a linked list node allocated with _dbus_list_alloc_link. + * Does not free the data in the node. + * + * @param link the list node + */ +void +_dbus_list_free_link (DBusList *link) +{ + free_link (link); +} + + +/** * Appends a value to the list. May return #FALSE * if insufficient memory exists to add a list link. * This is a constant-time operation. @@ -236,6 +271,43 @@ _dbus_list_prepend (DBusList **list, } /** + * Appends a link to the list. + * Cannot fail due to out of memory. + * This is a constant-time operation. + * + * @param list address of the list head. + * @param link the link to append. + */ +void +_dbus_list_append_link (DBusList **list, + DBusList *link) +{ + _dbus_list_prepend_link (list, link); + + /* Now cycle the list forward one so the prepended node is the tail */ + *list = (*list)->next; + + return TRUE; +} + +/** + * Prepends a link to the list. + * Cannot fail due to out of memory. + * This is a constant-time operation. + * + * @param list address of the list head. + * @param link the link to prepend. + */ +void +_dbus_list_prepend_link (DBusList **list, + DBusList *link) +{ + link_before (list, *list, link); + + return TRUE; +} + +/** * Inserts data into the list before the given existing link. * * @param list the list to modify diff --git a/dbus/dbus-list.h b/dbus/dbus-list.h index d1e3b74..2c55c6b 100644 --- a/dbus/dbus-list.h +++ b/dbus/dbus-list.h @@ -66,6 +66,13 @@ dbus_bool_t _dbus_list_copy (DBusList **list, DBusList **dest); int _dbus_list_get_length (DBusList **list); +DBusList* _dbus_list_alloc_link (void *data); +void _dbus_list_free_link (DBusList *link); +void _dbus_list_append_link (DBusList **list, + DBusList *link); +void _dbus_list_prepend_link (DBusList **list, + DBusList *link); + void _dbus_list_foreach (DBusList **list, DBusForeachFunction function, void *data); diff --git a/dbus/dbus-message-handler.c b/dbus/dbus-message-handler.c index 5f12d23..0f71d2e 100644 --- a/dbus/dbus-message-handler.c +++ b/dbus/dbus-message-handler.c @@ -24,6 +24,7 @@ #include "dbus-internals.h" #include "dbus-message-handler.h" #include "dbus-list.h" +#include "dbus-threads.h" #include "dbus-connection-internal.h" /** @@ -36,6 +37,14 @@ * @{ */ +static DBusMutex *message_handler_lock = NULL; +DBusMutex *_dbus_message_handler_init_lock (void); +DBusMutex * +_dbus_message_handler_init_lock (void) +{ + message_handler_lock = dbus_mutex_new (); + return message_handler_lock; +} /** * @brief Internals of DBusMessageHandler @@ -66,13 +75,20 @@ dbus_bool_t _dbus_message_handler_add_connection (DBusMessageHandler *handler, DBusConnection *connection) { + dbus_bool_t res; + + dbus_mutex_lock (message_handler_lock); /* This is a bit wasteful - we just put the connection in the list * once per time it's added. :-/ */ if (!_dbus_list_prepend (&handler->connections, connection)) - return FALSE; + res = FALSE; + else + res = TRUE; - return TRUE; + dbus_mutex_unlock (message_handler_lock); + + return res; } /** @@ -84,8 +100,10 @@ void _dbus_message_handler_remove_connection (DBusMessageHandler *handler, DBusConnection *connection) { + dbus_mutex_lock (message_handler_lock); if (!_dbus_list_remove (&handler->connections, connection)) _dbus_warn ("Function _dbus_message_handler_remove_connection() called when the connection hadn't been added\n"); + dbus_mutex_unlock (message_handler_lock); } @@ -104,11 +122,19 @@ _dbus_message_handler_handle_message (DBusMessageHandler *handler, DBusConnection *connection, DBusMessage *message) { + DBusHandleMessageFunction function; + void *user_data; + + dbus_mutex_lock (message_handler_lock); + function = handler->function; + user_data = handler->user_data; + dbus_mutex_unlock (message_handler_lock); + /* This function doesn't ref handler/connection/message * since that's done in dbus_connection_dispatch_message(). */ - if (handler->function != NULL) - return (* handler->function) (handler, connection, message, handler->user_data); + if (function != NULL) + return (* function) (handler, connection, message, user_data); else return DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS; } @@ -173,9 +199,11 @@ dbus_message_handler_new (DBusHandleMessageFunction function, void dbus_message_handler_ref (DBusMessageHandler *handler) { + dbus_mutex_lock (message_handler_lock); _dbus_assert (handler != NULL); handler->refcount += 1; + dbus_mutex_unlock (message_handler_lock); } /** @@ -187,11 +215,19 @@ dbus_message_handler_ref (DBusMessageHandler *handler) void dbus_message_handler_unref (DBusMessageHandler *handler) { + int refcount; + + dbus_mutex_lock (message_handler_lock); + _dbus_assert (handler != NULL); _dbus_assert (handler->refcount > 0); handler->refcount -= 1; - if (handler->refcount == 0) + refcount = handler->refcount; + + dbus_mutex_unlock (message_handler_lock); + + if (refcount == 0) { DBusList *link; @@ -203,7 +239,7 @@ dbus_message_handler_unref (DBusMessageHandler *handler) { DBusConnection *connection = link->data; - _dbus_connection_handler_destroyed (connection, handler); + _dbus_connection_handler_destroyed_locked (connection, handler); link = _dbus_list_get_next_link (&handler->connections, link); } @@ -224,7 +260,11 @@ dbus_message_handler_unref (DBusMessageHandler *handler) void* dbus_message_handler_get_data (DBusMessageHandler *handler) { - return handler->user_data; + void* user_data; + dbus_mutex_lock (message_handler_lock); + user_data = handler->user_data; + dbus_mutex_unlock (message_handler_lock); + return user_data; } /** @@ -241,11 +281,20 @@ dbus_message_handler_set_data (DBusMessageHandler *handler, void *user_data, DBusFreeFunction free_user_data) { - if (handler->free_user_data) - (* handler->free_user_data) (handler->user_data); + DBusFreeFunction old_free_func; + void *old_user_data; + + dbus_mutex_lock (message_handler_lock); + old_free_func = handler->free_user_data; + old_user_data = handler->user_data; handler->user_data = user_data; handler->free_user_data = free_user_data; + dbus_mutex_unlock (message_handler_lock); + + if (old_free_func) + (* old_free_func) (old_user_data); + } /** @@ -259,7 +308,9 @@ void dbus_message_handler_set_function (DBusMessageHandler *handler, DBusHandleMessageFunction function) { + dbus_mutex_lock (message_handler_lock); handler->function = function; + dbus_mutex_unlock (message_handler_lock); } /** @} */ diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 91ff29d..aca8c2c 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -81,7 +81,7 @@ typedef struct */ struct DBusMessage { - int refcount; /**< Reference count */ + dbus_atomic_t refcount; /**< Reference count */ DBusString header; /**< Header network data, stored * separately from body so we can @@ -887,9 +887,10 @@ dbus_message_new_from_message (const DBusMessage *message) void dbus_message_ref (DBusMessage *message) { - _dbus_assert (message->refcount > 0); - - message->refcount += 1; + dbus_atomic_t refcount; + + refcount = _dbus_atomic_inc (&message->refcount); + _dbus_assert (refcount > 1); } /** @@ -901,10 +902,13 @@ dbus_message_ref (DBusMessage *message) void dbus_message_unref (DBusMessage *message) { - _dbus_assert (message->refcount > 0); + dbus_atomic_t refcount; - message->refcount -= 1; - if (message->refcount == 0) + refcount = _dbus_atomic_dec (&message->refcount); + + _dbus_assert (refcount >= 0); + + if (refcount == 0) { if (message->size_counter != NULL) { @@ -1519,7 +1523,7 @@ dbus_message_iter_get_string (DBusMessageIter *iter) /** * Returns the 32 bit signed integer value that an iterator may point to. * Note that you need to check that the iterator points to - * a string value before using this function. + * an integer value before using this function. * * @see dbus_message_iter_get_field_type * @param iter the message iter @@ -1535,7 +1539,7 @@ dbus_message_iter_get_int32 (DBusMessageIter *iter) /** * Returns the 32 bit unsigned integer value that an iterator may point to. * Note that you need to check that the iterator points to - * a string value before using this function. + * an unsigned integer value before using this function. * * @see dbus_message_iter_get_field_type * @param iter the message iter diff --git a/dbus/dbus-protocol.h b/dbus/dbus-protocol.h index be96b26..f043346 100644 --- a/dbus/dbus-protocol.h +++ b/dbus/dbus-protocol.h @@ -85,6 +85,7 @@ extern "C" { #define DBUS_MESSAGE_SERVICE_DELETED "org.freedesktop.DBus.ServiceDeleted" #define DBUS_MESSAGE_SERVICE_LOST "org.freedesktop.DBus.ServiceLost" +#define DBUS_MESSAGE_LOCAL_DISCONNECT "org.freedesktop.Local.Disconnect" #ifdef __cplusplus } diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c index 38efb0c..53eebf7 100644 --- a/dbus/dbus-sysdeps.c +++ b/dbus/dbus-sysdeps.c @@ -23,6 +23,7 @@ #include "dbus-internals.h" #include "dbus-sysdeps.h" +#include "dbus-threads.h" #include #include #include @@ -941,6 +942,55 @@ _dbus_string_append_our_uid (DBusString *str) } +static DBusMutex *atomic_lock = NULL; +DBusMutex *_dbus_atomic_init_lock (void); +DBusMutex * +_dbus_atomic_init_lock (void) +{ + atomic_lock = dbus_mutex_new (); + return atomic_lock; +} + +/** + * Atomically increments an integer + * + * @param atomic pointer to the integer to increment + * @returns the value after incrementing + * + * @todo implement arch-specific faster atomic ops + */ +dbus_atomic_t +_dbus_atomic_inc (dbus_atomic_t *atomic) +{ + dbus_atomic_t res; + + dbus_mutex_lock (atomic_lock); + *atomic += 1; + res = *atomic; + dbus_mutex_unlock (atomic_lock); + return res; +} + +/** + * Atomically decrement an integer + * + * @param atomic pointer to the integer to decrement + * @returns the value after decrementing + * + * @todo implement arch-specific faster atomic ops + */ +dbus_atomic_t +_dbus_atomic_dec (dbus_atomic_t *atomic) +{ + dbus_atomic_t res; + + dbus_mutex_lock (atomic_lock); + *atomic -= 1; + res = *atomic; + dbus_mutex_unlock (atomic_lock); + return res; +} + /** * Wrapper for poll(). * diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index 3212a23..76c943b 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -98,6 +98,10 @@ dbus_bool_t _dbus_credentials_match (const DBusCredentials *expec dbus_bool_t _dbus_string_append_our_uid (DBusString *str); +typedef int dbus_atomic_t; + +dbus_atomic_t _dbus_atomic_inc (dbus_atomic_t *atomic); +dbus_atomic_t _dbus_atomic_dec (dbus_atomic_t *atomic); #define _DBUS_POLLIN 0x0001 /* There is data to read */ #define _DBUS_POLLPRI 0x0002 /* There is urgent data to read */ diff --git a/dbus/dbus-threads.c b/dbus/dbus-threads.c index b132e8a..4df2e34 100644 --- a/dbus/dbus-threads.c +++ b/dbus/dbus-threads.c @@ -27,16 +27,18 @@ static DBusThreadFunctions thread_functions = { 0, NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL }; -static DBusMutex *static_mutex_init_lock = NULL; - /** This is used for the no-op default mutex pointer, just to be distinct from #NULL */ #define _DBUS_DUMMY_MUTEX ((void*)0xABCDEF) +/** This is used for the no-op default mutex pointer, just to be distinct from #NULL */ +#define _DBUS_DUMMY_CONDVAR ((void*)0xABCDEF2) + /** * @defgroup DBusThreads Thread functions * @ingroup DBus @@ -105,6 +107,130 @@ dbus_mutex_unlock (DBusMutex *mutex) } /** + * Creates a new condition variable using the function supplied + * to dbus_threads_init(), or creates a no-op condition variable + * if threads are not initialized. May return #NULL even if + * threads are initialized, indicating out-of-memory. + * + * @returns new mutex or #NULL + */ +DBusCondVar * +dbus_condvar_new (void) +{ + if (thread_functions.condvar_new) + return (* thread_functions.condvar_new) (); + else + return _DBUS_DUMMY_MUTEX; +} + +/** + * Frees a conditional variable created with dbus_condvar_new(); does + * nothing if passed a #NULL pointer. + */ +void +dbus_condvar_free (DBusCondVar *cond) +{ + if (cond && thread_functions.condvar_free) + (* thread_functions.condvar_free) (cond); +} + +/** + * Atomically unlocks the mutex and waits for the conditions + * variable to be signalled. Locks the mutex again before + * returning. + * Does nothing if passed a #NULL pointer. + */ +void +dbus_condvar_wait (DBusCondVar *cond, + DBusMutex *mutex) +{ + if (cond && mutex && thread_functions.condvar_wait) + (* thread_functions.condvar_wait) (cond, mutex); +} + +/** + * Atomically unlocks the mutex and waits for the conditions + * variable to be signalled, or for a timeout. Locks the + * mutex again before returning. + * Does nothing if passed a #NULL pointer. + * + * @param timeout_milliseconds the maximum time to wait + * @returns TRUE if the condition was reached, or FALSE if the + * timeout was reached. + */ +dbus_bool_t +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); + else + return FALSE; +} + +/** + * If there are threads waiting on the condition variable, wake + * up exactly one. + * Does nothing if passed a #NULL pointer. + */ +void +dbus_condvar_wake_one (DBusCondVar *cond) +{ + if (cond && thread_functions.condvar_wake_one) + (* thread_functions.condvar_wake_one) (cond); +} + +/** + * If there are threads waiting on the condition variable, wake + * up all of them. + * Does nothing if passed a #NULL pointer. + */ +void +dbus_condvar_wake_all (DBusCondVar *cond) +{ + if (cond && thread_functions.condvar_wake_all) + (* thread_functions.condvar_wake_all) (cond); +} + + +DBusMutex * _dbus_list_init_lock (void); +DBusMutex * _dbus_allocated_slots_init_lock (void); +DBusMutex *_dbus_atomic_init_lock (void); +DBusMutex *_dbus_message_handler_init_lock (void); + +static dbus_bool_t +init_static_locks(void) +{ + int i; + + struct { + DBusMutex *(*init_func)(void); + DBusMutex *mutex; + } static_locks[] = { + {&_dbus_list_init_lock}, + {&_dbus_allocated_slots_init_lock}, + {&_dbus_atomic_init_lock}, + {&_dbus_message_handler_init_lock}, + }; + + for (i = 0; i < _DBUS_N_ELEMENTS (static_locks); i++) + { + static_locks[i].mutex = (*static_locks[i].init_func)(); + + if (static_locks[i].mutex == NULL) + { + for (i = i - 1; i >= 0; i--) + dbus_mutex_free (static_locks[i].mutex); + return FALSE; + } + + } + return TRUE; +} + + +/** * 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 @@ -125,14 +251,26 @@ dbus_threads_init (const DBusThreadFunctions *functions) /* these base functions are required. Future additions to * DBusThreadFunctions may be optional. */ - _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_NEW_MASK); - _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_FREE_MASK); - _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_LOCK_MASK); - _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_UNLOCK_MASK); + _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_NEW_MASK); + _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_FREE_MASK); + _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_LOCK_MASK); + _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_MUTEX_UNLOCK_MASK); + _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_NEW_MASK); + _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_FREE_MASK); + _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_MASK); + _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_TIMEOUT_MASK); + _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ONE_MASK); + _dbus_assert (functions->mask & DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ALL_MASK); _dbus_assert (functions->mutex_new != NULL); _dbus_assert (functions->mutex_free != NULL); _dbus_assert (functions->mutex_lock != NULL); _dbus_assert (functions->mutex_unlock != NULL); + _dbus_assert (functions->condvar_new != NULL); + _dbus_assert (functions->condvar_free != NULL); + _dbus_assert (functions->condvar_wait != NULL); + _dbus_assert (functions->condvar_wait_timeout != NULL); + _dbus_assert (functions->condvar_wake_one != NULL); + _dbus_assert (functions->condvar_wake_all != NULL); /* Check that all bits in the mask actually are valid mask bits. * ensures people won't write code that breaks when we add @@ -151,67 +289,19 @@ dbus_threads_init (const DBusThreadFunctions *functions) thread_functions.mutex_lock = functions->mutex_lock; thread_functions.mutex_unlock = functions->mutex_unlock; - thread_functions.mask = functions->mask; - - static_mutex_init_lock = dbus_mutex_new (); - - if (static_mutex_init_lock == NULL) - { - thread_functions.mask = 0; - return FALSE; - } - - return TRUE; -} - -/** Accesses the field of DBusStaticMutex that - * stores the DBusMutex used to implement. - */ -#define _DBUS_STATIC_MUTEX_IMPL(mutex) ((mutex)->pad1) - -/** - * Lock a static mutex - * - * @todo currently broken on some platforms due to - * non-workingness of "double checked locking" - * see http://bugzilla.gnome.org/show_bug.cgi?id=69668 - * and http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html - * for example. - * - * @param mutex the mutex to lock - * @returns #TRUE on success - */ -dbus_bool_t -dbus_static_mutex_lock (DBusStaticMutex *mutex) -{ - if (_DBUS_STATIC_MUTEX_IMPL (mutex)) - return dbus_mutex_lock (_DBUS_STATIC_MUTEX_IMPL (mutex)); - - if (!dbus_mutex_lock (static_mutex_init_lock)) - return FALSE; - - if (_DBUS_STATIC_MUTEX_IMPL (mutex) == NULL) - _DBUS_STATIC_MUTEX_IMPL (mutex) = dbus_mutex_new (); + thread_functions.condvar_new = functions->condvar_new; + thread_functions.condvar_free = functions->condvar_free; + thread_functions.condvar_wait = functions->condvar_wait; + thread_functions.condvar_wait_timeout = functions->condvar_wait_timeout; + thread_functions.condvar_wake_one = functions->condvar_wake_one; + thread_functions.condvar_wake_all = functions->condvar_wake_all; - dbus_mutex_unlock (static_mutex_init_lock); + thread_functions.mask = functions->mask; - if (_DBUS_STATIC_MUTEX_IMPL (mutex)) - return dbus_mutex_lock (_DBUS_STATIC_MUTEX_IMPL (mutex)); - else + if (!init_static_locks ()) return FALSE; -} - -/** - * Unlock a static mutex - * @param mutex the mutex to lock - * @returns #TRUE on success - */ -dbus_bool_t -dbus_static_mutex_unlock (DBusStaticMutex *mutex) -{ - _dbus_assert (_DBUS_STATIC_MUTEX_IMPL (mutex) != NULL); - return dbus_mutex_unlock (_DBUS_STATIC_MUTEX_IMPL (mutex)); + return TRUE; } /** @} */ diff --git a/dbus/dbus-threads.h b/dbus/dbus-threads.h index ddc270b..0dcb104 100644 --- a/dbus/dbus-threads.h +++ b/dbus/dbus-threads.h @@ -33,20 +33,37 @@ DBUS_BEGIN_DECLS; typedef struct DBusMutex DBusMutex; +typedef struct DBusCondVar DBusCondVar; typedef DBusMutex* (* DBusMutexNewFunction) (void); typedef void (* DBusMutexFreeFunction) (DBusMutex *mutex); typedef dbus_bool_t (* DBusMutexLockFunction) (DBusMutex *mutex); typedef dbus_bool_t (* DBusMutexUnlockFunction) (DBusMutex *mutex); +typedef DBusCondVar* (* DBusCondVarNewFunction) (void); +typedef void (* DBusCondVarFreeFunction) (DBusCondVar *cond); +typedef void (* DBusCondVarWaitFunction) (DBusCondVar *cond, + DBusMutex *mutex); +typedef dbus_bool_t (* DBusCondVarWaitTimeoutFunction) (DBusCondVar *cond, + DBusMutex *mutex, + int timeout_milliseconds); +typedef void (* DBusCondVarWakeOneFunction) (DBusCondVar *cond); +typedef void (* DBusCondVarWakeAllFunction) (DBusCondVar *cond); + typedef enum { - DBUS_THREAD_FUNCTIONS_NEW_MASK = 1 << 0, - DBUS_THREAD_FUNCTIONS_FREE_MASK = 1 << 1, - DBUS_THREAD_FUNCTIONS_LOCK_MASK = 1 << 2, - DBUS_THREAD_FUNCTIONS_UNLOCK_MASK = 1 << 3, - - DBUS_THREAD_FUNCTIONS_ALL_MASK = 0xf + DBUS_THREAD_FUNCTIONS_MUTEX_NEW_MASK = 1 << 0, + DBUS_THREAD_FUNCTIONS_MUTEX_FREE_MASK = 1 << 1, + DBUS_THREAD_FUNCTIONS_MUTEX_LOCK_MASK = 1 << 2, + DBUS_THREAD_FUNCTIONS_MUTEX_UNLOCK_MASK = 1 << 3, + DBUS_THREAD_FUNCTIONS_CONDVAR_NEW_MASK = 1 << 4, + DBUS_THREAD_FUNCTIONS_CONDVAR_FREE_MASK = 1 << 5, + DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_MASK = 1 << 6, + DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_TIMEOUT_MASK = 1 << 7, + DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ONE_MASK = 1 << 8, + DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ALL_MASK = 1 << 9, + + DBUS_THREAD_FUNCTIONS_ALL_MASK = (1 << 10) - 1 } DBusThreadFunctionsMask; typedef struct @@ -58,6 +75,13 @@ typedef struct DBusMutexLockFunction mutex_lock; DBusMutexUnlockFunction mutex_unlock; + DBusCondVarNewFunction condvar_new; + DBusCondVarFreeFunction condvar_free; + DBusCondVarWaitFunction condvar_wait; + DBusCondVarWaitTimeoutFunction condvar_wait_timeout; + DBusCondVarWakeOneFunction condvar_wake_one; + DBusCondVarWakeAllFunction condvar_wake_all; + void (* padding1) (void); void (* padding2) (void); void (* padding3) (void); @@ -70,27 +94,24 @@ 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); +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); -dbus_bool_t dbus_threads_init (const DBusThreadFunctions *functions); +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); -typedef struct DBusStaticMutex DBusStaticMutex; - -struct DBusStaticMutex -{ - void *pad1; - void *pad2; - void *pad3; - void *pad4; -}; +dbus_bool_t dbus_threads_init (const DBusThreadFunctions *functions); -#define DBUS_STATIC_MUTEX_INIT { NULL, NULL, NULL, NULL } -dbus_bool_t dbus_static_mutex_lock (DBusStaticMutex *mutex); -dbus_bool_t dbus_static_mutex_unlock (DBusStaticMutex *mutex); DBUS_END_DECLS; diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index a986081..5a25a67 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -52,31 +52,13 @@ * or encryption schemes. */ -/** - * Refs a transport and associated connection for reentrancy. - * - * @todo this macro reflects a design mistake, which is that the - * transport has a pointer to its connection. Ownership should move in - * only one direction; the connection should push/pull from the - * transport, rather than vice versa. Then the connection would take - * care of referencing itself when needed. - */ -#define DBUS_TRANSPORT_HOLD_REF(t) \ - _dbus_transport_ref (t); if ((t)->connection) dbus_connection_ref ((t)->connection) - -/** - * Inverse of DBUS_TRANSPORT_HOLD_REF(). - */ -#define DBUS_TRANSPORT_RELEASE_REF(t) \ - if ((t)->connection) dbus_connection_unref ((t)->connection); _dbus_transport_unref (t) - static void live_messages_size_notify (DBusCounter *counter, void *user_data) { DBusTransport *transport = user_data; - DBUS_TRANSPORT_HOLD_REF (transport); + _dbus_transport_ref (transport); #if 0 _dbus_verbose ("Counter value is now %d\n", @@ -89,7 +71,7 @@ live_messages_size_notify (DBusCounter *counter, if (* transport->vtable->live_messages_changed) (* transport->vtable->live_messages_changed) (transport); - DBUS_TRANSPORT_RELEASE_REF (transport); + _dbus_transport_unref (transport); } /** @@ -294,14 +276,14 @@ _dbus_transport_disconnect (DBusTransport *transport) if (transport->disconnected) return; - DBUS_TRANSPORT_HOLD_REF (transport); + _dbus_transport_ref (transport); (* transport->vtable->disconnect) (transport); transport->disconnected = TRUE; _dbus_connection_notify_disconnected (transport->connection); - DBUS_TRANSPORT_RELEASE_REF (transport); + _dbus_transport_unref (transport); } /** @@ -401,11 +383,11 @@ _dbus_transport_handle_watch (DBusTransport *transport, _dbus_watch_sanitize_condition (watch, &condition); - DBUS_TRANSPORT_HOLD_REF (transport); + _dbus_transport_ref (transport); _dbus_watch_ref (watch); (* transport->vtable->handle_watch) (transport, watch, condition); _dbus_watch_unref (watch); - DBUS_TRANSPORT_RELEASE_REF (transport); + _dbus_transport_unref (transport); } /** @@ -425,9 +407,9 @@ _dbus_transport_set_connection (DBusTransport *transport, transport->connection = connection; - DBUS_TRANSPORT_HOLD_REF (transport); + _dbus_transport_ref (transport); (* transport->vtable->connection_set) (transport); - DBUS_TRANSPORT_RELEASE_REF (transport); + _dbus_transport_unref (transport); } /** @@ -450,10 +432,10 @@ _dbus_transport_messages_pending (DBusTransport *transport, transport->messages_need_sending = queue_length > 0; - DBUS_TRANSPORT_HOLD_REF (transport); + _dbus_transport_ref (transport); (* transport->vtable->messages_pending) (transport, queue_length); - DBUS_TRANSPORT_RELEASE_REF (transport); + _dbus_transport_unref (transport); } /** @@ -481,10 +463,10 @@ _dbus_transport_do_iteration (DBusTransport *transport, if (transport->disconnected) return; - DBUS_TRANSPORT_HOLD_REF (transport); + _dbus_transport_ref (transport); (* transport->vtable->do_iteration) (transport, flags, timeout_milliseconds); - DBUS_TRANSPORT_RELEASE_REF (transport); + _dbus_transport_unref (transport); } /** diff --git a/glib/dbus-gmain.c b/glib/dbus-gmain.c index 80d7d6b..857ee37 100644 --- a/glib/dbus-gmain.c +++ b/glib/dbus-gmain.c @@ -59,7 +59,7 @@ dbus_connection_prepare (GSource *source, *timeout = -1; - return (dbus_connection_peek_message (connection) != NULL); + return (dbus_connection_get_n_messages (connection) > 0); } static gboolean diff --git a/glib/dbus-gthread.c b/glib/dbus-gthread.c index 685f4cd..8ed0a13 100644 --- a/glib/dbus-gthread.c +++ b/glib/dbus-gthread.c @@ -25,21 +25,45 @@ #include #include "dbus-glib.h" -static DBusMutex * dbus_gmutex_new (void); -static void dbus_gmutex_free (DBusMutex *mutex); -static dbus_bool_t dbus_gmutex_lock (DBusMutex *mutex); -static dbus_bool_t dbus_gmutex_unlock (DBusMutex *mutex); +static DBusMutex * dbus_gmutex_new (void); +static void dbus_gmutex_free (DBusMutex *mutex); +static dbus_bool_t dbus_gmutex_lock (DBusMutex *mutex); +static dbus_bool_t dbus_gmutex_unlock (DBusMutex *mutex); + + +static DBusCondVar* dbus_gcondvar_new (void); +static void dbus_gcondvar_free (DBusCondVar *cond); +static void dbus_gcondvar_wait (DBusCondVar *cond, + DBusMutex *mutex); +static dbus_bool_t dbus_gcondvar_wait_timeout (DBusCondVar *cond, + DBusMutex *mutex, + int timeout_msec); +static void dbus_gcondvar_wake_one (DBusCondVar *cond); +static void dbus_gcondvar_wake_all (DBusCondVar *cond); + static const DBusThreadFunctions functions = { - DBUS_THREAD_FUNCTIONS_NEW_MASK | - DBUS_THREAD_FUNCTIONS_FREE_MASK | - DBUS_THREAD_FUNCTIONS_LOCK_MASK | - DBUS_THREAD_FUNCTIONS_UNLOCK_MASK, + DBUS_THREAD_FUNCTIONS_MUTEX_NEW_MASK | + DBUS_THREAD_FUNCTIONS_MUTEX_FREE_MASK | + DBUS_THREAD_FUNCTIONS_MUTEX_LOCK_MASK | + DBUS_THREAD_FUNCTIONS_MUTEX_UNLOCK_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_NEW_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_FREE_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_TIMEOUT_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ONE_MASK| + DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ALL_MASK, dbus_gmutex_new, dbus_gmutex_free, dbus_gmutex_lock, - dbus_gmutex_unlock + dbus_gmutex_unlock, + dbus_gcondvar_new, + dbus_gcondvar_free, + dbus_gcondvar_wait, + dbus_gcondvar_wait_timeout, + dbus_gcondvar_wake_one, + dbus_gcondvar_wake_all }; static DBusMutex * @@ -74,6 +98,58 @@ dbus_gmutex_unlock (DBusMutex *mutex) return TRUE; } +static DBusCondVar* +dbus_gcondvar_new (void) +{ + return (DBusCondVar*)g_cond_new (); +} + +static void +dbus_gcondvar_free (DBusCondVar *cond) +{ + g_cond_free ((GCond *)cond); +} + +static void +dbus_gcondvar_wait (DBusCondVar *cond, + DBusMutex *mutex) +{ + g_cond_wait ((GCond *)cond, (GMutex *)mutex); +} + +static dbus_bool_t +dbus_gcondvar_wait_timeout (DBusCondVar *cond, + DBusMutex *mutex, + int timeout_msec) +{ + GTimeVal now; + + g_get_current_time (&now); + + now.tv_sec += timeout_msec / 1000; + now.tv_usec += (timeout_msec % 1000) * 1000; + if (now.tv_usec > G_USEC_PER_SEC) + { + now.tv_sec += 1; + now.tv_usec -= G_USEC_PER_SEC; + } + + return g_cond_timed_wait ((GCond *)cond, (GMutex *)mutex, &now); +} + +static void +dbus_gcondvar_wake_one (DBusCondVar *cond) +{ + g_cond_signal ((GCond *)cond); +} + +static void +dbus_gcondvar_wake_all (DBusCondVar *cond) +{ + g_cond_broadcast ((GCond *)cond); +} + + void dbus_gthread_init (void) { diff --git a/qt/dbus-qthread.cpp b/qt/dbus-qthread.cpp index 5aa0fa1..840f16e 100644 --- a/qt/dbus-qthread.cpp +++ b/qt/dbus-qthread.cpp @@ -31,17 +31,39 @@ static void dbus_qmutex_free (DBusMutex *mutex); static dbus_bool_t dbus_qmutex_lock (DBusMutex *mutex); static dbus_bool_t dbus_qmutex_unlock (DBusMutex *mutex); +static DBusCondVar*dbus_qcondvar_new (void); +static void dbus_qcondvar_free (DBusCondVar *cond); +static void dbus_qcondvar_wait (DBusCondVar *cond, + DBusMutex *mutex); +static dbus_bool_t dbus_qcondvar_wait_timeout (DBusCondVar *cond, + DBusMutex *mutex. + int timeout_msec); +static void dbus_qcondvar_wake_one (DBusCondVar *cond); +static void dbus_qcondvar_wake_all (DBusCondVar *cond); + static const DBusThreadFunctions functions = { DBUS_THREAD_FUNCTIONS_NEW_MASK | DBUS_THREAD_FUNCTIONS_FREE_MASK | DBUS_THREAD_FUNCTIONS_LOCK_MASK | - DBUS_THREAD_FUNCTIONS_UNLOCK_MASK, + DBUS_THREAD_FUNCTIONS_UNLOCK_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_NEW_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_FREE_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_TIMEOUT_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ONE_MASK| + DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ALL_MASK, dbus_qmutex_new, dbus_qmutex_free, dbus_qmutex_lock, dbus_qmutex_unlock + dbus_qcondvar_new, + dbus_qcondvar_free, + dbus_qcondvar_wait, + dbus_qcondvar_wait_timeout, + dbus_qcondvar_wake_one, + dbus_qcondvar_wake_all }; static DBusMutex * @@ -75,6 +97,58 @@ dbus_qmutex_unlock (DBusMutex *mutex) return TRUE; } +static DBusCondVar* +dbus_qcondvar_new (void) +{ + QWaitCondition *cond; + cond = new QWaitCondition; + return static_cast( cond ); +} + +static void +dbus_qcondvar_free (DBusCondVar *cond) +{ + QWaitCondition *cqond = static_cast(mutex); + delete qcond; +} + +static void +dbus_qcondvar_wait (DBusCondVar *cond, + DBusMutex *mutex) +{ + QWaitCondition *qcond = static_cast(cond); + QMutex *qmutex = static_cast(mutex); + + qcond->wait (qmutex); +} + +static dbus_bool_t +dbus_gcondvar_wait_timeout (DBusCondVar *cond, + DBusMutex *mutex, + int timeout_msec) +{ + QWaitCondition *qcond = static_cast(cond); + QMutex *qmutex = static_cast(mutex); + + return qcond->wait (qmutex, timout_msec); +} + +static void +dbus_qcondvar_wake_one (DBusCondVar *cond) +{ + QWaitCondition *qcond = static_cast(cond); + + qcond->wakeOne (qmutex); +} + +static void +dbus_qcondvar_wake_all (DBusCondVar *cond) +{ + QWaitCondition *qcond = static_cast(cond); + + qcond->wakeAll (qmutex); +} + extern "C" { void diff --git a/test/Makefile.am b/test/Makefile.am index 4ff6c49..ad65782 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -24,6 +24,8 @@ unbase64_SOURCES= \ bus_test_SOURCES = \ + debug-thread.c \ + debug-thread.h \ bus-test.c break_loader_SOURCES= \ diff --git a/test/bus-test.c b/test/bus-test.c index 624c11e..3874280 100644 --- a/test/bus-test.c +++ b/test/bus-test.c @@ -8,6 +8,8 @@ #undef DBUS_COMPILATION +#include "debug-thread.h" + typedef struct { long time; @@ -140,6 +142,8 @@ main (int argc, DBusMessage *message; DBusMessageHandler *handler; + debug_threads_init (); + server = dbus_server_listen ("debug:name=test-server", &result); dbus_server_set_new_connection_function (server, new_connection_callback, diff --git a/test/debug-thread.c b/test/debug-thread.c new file mode 100644 index 0000000..5ef3ba4 --- /dev/null +++ b/test/debug-thread.c @@ -0,0 +1,161 @@ +/* -*- mode: C; c-file-style: "gnu" -*- */ +/* dbus-test.c Program to run all tests + * + * Copyright (C) 2002 Red Hat Inc. + * + * Licensed under the Academic Free License version 1.2 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include +#include +#include "debug-thread.h" + +#define DBUS_COMPILATION +#include +#undef DBUS_COMPILATION + + +static DBusMutex * tmutex_new (void); +static void tmutex_free (DBusMutex *mutex); +static dbus_bool_t tmutex_lock (DBusMutex *mutex); +static dbus_bool_t tmutex_unlock (DBusMutex *mutex); + +static DBusCondVar*tcondvar_new (void); +static void tcondvar_free (DBusCondVar *cond); +static void tcondvar_wait (DBusCondVar *cond, + DBusMutex *mutex); +static dbus_bool_t tcondvar_wait_timeout (DBusCondVar *cond, + DBusMutex *mutex, + int timeout_msec); +static void tcondvar_wake_one (DBusCondVar *cond); +static void tcondvar_wake_all (DBusCondVar *cond); + +static const DBusThreadFunctions functions = +{ + DBUS_THREAD_FUNCTIONS_MUTEX_NEW_MASK | + DBUS_THREAD_FUNCTIONS_MUTEX_FREE_MASK | + DBUS_THREAD_FUNCTIONS_MUTEX_LOCK_MASK | + DBUS_THREAD_FUNCTIONS_MUTEX_UNLOCK_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_NEW_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_FREE_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_TIMEOUT_MASK | + DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ONE_MASK| + DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ALL_MASK, + tmutex_new, + tmutex_free, + tmutex_lock, + tmutex_unlock, + tcondvar_new, + tcondvar_free, + tcondvar_wait, + tcondvar_wait_timeout, + tcondvar_wake_one, + tcondvar_wake_all +}; + +static DBusMutex * +tmutex_new (void) +{ + int *tmutex; + + tmutex = malloc (sizeof (int*)); + *tmutex = 0; + + return (DBusMutex *)tmutex; +} + +static void +tmutex_free (DBusMutex *mutex) +{ + free (mutex); +} + +static dbus_bool_t +tmutex_lock (DBusMutex *mutex) +{ + int *tmutex = (int *)mutex; + + _dbus_assert (*tmutex == 0); + + *tmutex = 1; + + return TRUE; +} + +static dbus_bool_t +tmutex_unlock (DBusMutex *mutex) +{ + int *tmutex = (int *)mutex; + + _dbus_assert (*tmutex == 1); + + *tmutex = 0; + + return TRUE; +} + +static DBusCondVar* +tcondvar_new (void) +{ + return (DBusCondVar*)0xcafebabe; +} + +static void +tcondvar_free (DBusCondVar *cond) +{ +} + +static void +tcondvar_wait (DBusCondVar *cond, + DBusMutex *mutex) +{ + int *tmutex = (int *)mutex; + + _dbus_assert (*tmutex == 1); +} + +static dbus_bool_t +tcondvar_wait_timeout (DBusCondVar *cond, + DBusMutex *mutex, + int timeout_msec) +{ + int *tmutex = (int *)mutex; + + _dbus_assert (*tmutex == 1); + + return TRUE; +} + + +static void +tcondvar_wake_one (DBusCondVar *cond) +{ +} + +static void +tcondvar_wake_all (DBusCondVar *cond) +{ +} + +void +debug_threads_init (void) +{ + dbus_threads_init (&functions); +} + diff --git a/test/debug-thread.h b/test/debug-thread.h new file mode 100644 index 0000000..57adff8 --- /dev/null +++ b/test/debug-thread.h @@ -0,0 +1,29 @@ +/* -*- mode: C; c-file-style: "gnu" -*- */ +/* dbus-test.c Program to run all tests + * + * Copyright (C) 2002 Red Hat Inc. + * + * Licensed under the Academic Free License version 1.2 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef DEBUG_THREAD_H +#define DEBUG_THREAD_H + +void debug_threads_init (void); + +#endif diff --git a/test/watch.c b/test/watch.c index a885a75..1a31e64 100644 --- a/test/watch.c +++ b/test/watch.c @@ -131,6 +131,17 @@ remove_server_watch (DBusWatch *watch, static int count = 0; static void +disconnect (DBusConnection *connection) +{ + fprintf (stderr, "Disconnected\n"); + + _dbus_list_remove (&connections, connection); + dbus_connection_unref (connection); + quit_mainloop (); +} + + +static void check_messages (void) { DBusList *link; @@ -141,28 +152,37 @@ check_messages (void) DBusList *next = _dbus_list_get_next_link (&connections, link); DBusConnection *connection = link->data; DBusMessage *message; + const char *name; while ((message = dbus_connection_pop_message (connection))) { DBusMessage *reply; - fprintf (stderr, "Received message %d, sending reply\n", count); - - reply = dbus_message_new ("org.freedesktop.DBus.Test", "org.freedesktop.DBus.Test"); - dbus_connection_send_message (connection, - reply, - NULL, - NULL); - dbus_message_unref (reply); - - dbus_message_unref (message); - - count += 1; - if (count > 100) - { - printf ("Saw %d messages, exiting\n", count); - quit_mainloop (); - } + name = dbus_message_get_name (message); + if (name && strcmp (name, DBUS_MESSAGE_LOCAL_DISCONNECT) == 0) + { + disconnect (connection); + } + else + { + fprintf (stderr, "Received message %d, sending reply\n", count); + + reply = dbus_message_new ("org.freedesktop.DBus.Test", "org.freedesktop.DBus.Test"); + dbus_connection_send_message (connection, + reply, + NULL, + NULL); + dbus_message_unref (reply); + + dbus_message_unref (message); + + count += 1; + if (count > 100) + { + printf ("Saw %d messages, exiting\n", count); + quit_mainloop (); + } + } } link = next; @@ -185,6 +205,9 @@ do_mainloop (void) int initial_watch_serial; check_messages (); + + if (exited) + break; FD_ZERO (&read_set); FD_ZERO (&write_set); @@ -297,16 +320,6 @@ quit_mainloop (void) exited = TRUE; } -static void -disconnect_handler (DBusConnection *connection, - void *data) -{ - fprintf (stderr, "Disconnected\n"); - - _dbus_list_remove (&connections, connection); - dbus_connection_unref (connection); - quit_mainloop (); -} void setup_connection (DBusConnection *connection) @@ -317,10 +330,6 @@ setup_connection (DBusConnection *connection) connection, NULL); - dbus_connection_set_disconnect_function (connection, - disconnect_handler, - NULL, NULL); - dbus_connection_ref (connection); _dbus_list_append (&connections, connection); }