dbus/dbus dbus-connection.c,1.96,1.97 dbus-internals.c,1.42,1.43
Havoc Pennington
hp at freedesktop.org
Sun Feb 13 12:22:01 PST 2005
Update of /cvs/dbus/dbus/dbus
In directory gabe:/tmp/cvs-serv9190/dbus
Modified Files:
dbus-connection.c dbus-internals.c
Log Message:
2005-02-13 Havoc Pennington <hp at redhat.com>
* dbus/dbus-connection.c (dbus_connection_return_message)
(dbus_connection_borrow_message): hold dispatch lock while message
is outstanding
(_dbus_connection_block_for_reply): hold dispatch lock while we
block for the reply, so nobody steals our reply
(dbus_connection_pop_message): hold the dispatch lock while we
pluck the message
Index: dbus-connection.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection.c,v
retrieving revision 1.96
retrieving revision 1.97
diff -u -d -r1.96 -r1.97
--- dbus-connection.c 13 Feb 2005 19:49:22 -0000 1.96
+++ dbus-connection.c 13 Feb 2005 20:21:59 -0000 1.97
@@ -196,8 +196,9 @@
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; /**< Used with dbus_connection_borrow_message() */
+ DBusMessage *message_borrowed; /**< Filled in if the first incoming message has been borrowed;
+ * dispatch_acquired will be set by the borrower
+ */
int n_outgoing; /**< Length of outgoing queue. */
int n_incoming; /**< Length of incoming queue. */
@@ -232,8 +233,8 @@
*/
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 dispatch_acquired : 1; /**< Someone has dispatch path (can drain incoming queue) */
+ unsigned int io_path_acquired : 1; /**< Someone has transport io path (can use the transport to read/write messages) */
unsigned int exit_on_disconnect : 1; /**< If #TRUE, exit after handling disconnect signal */
@@ -250,6 +251,8 @@
static void _dbus_connection_update_dispatch_status_and_unlock (DBusConnection *connection,
DBusDispatchStatus new_status);
static void _dbus_connection_last_unref (DBusConnection *connection);
+static void _dbus_connection_acquire_dispatch (DBusConnection *connection);
+static void _dbus_connection_release_dispatch (DBusConnection *connection);
static DBusMessageFilter *
_dbus_message_filter_ref (DBusMessageFilter *filter)
@@ -1162,7 +1165,6 @@
connection->dispatch_mutex = dispatch_mutex;
connection->io_path_cond = io_path_cond;
connection->io_path_mutex = io_path_mutex;
- connection->message_returned_cond = message_returned_cond;
connection->transport = transport;
connection->watches = watch_list;
connection->timeouts = timeout_list;
@@ -1540,7 +1542,6 @@
dbus_condvar_free (connection->dispatch_cond);
dbus_condvar_free (connection->io_path_cond);
- dbus_condvar_free (connection->message_returned_cond);
dbus_mutex_free (connection->io_path_mutex);
dbus_mutex_free (connection->dispatch_mutex);
@@ -2177,6 +2178,9 @@
DBusList *link;
HAVE_LOCK_CHECK (connection);
+
+ /* popping incoming_messages requires dispatch lock */
+ _dbus_assert (connection->dispatch_acquired);
link = _dbus_list_get_first_link (&connection->incoming_messages);
@@ -2202,7 +2206,14 @@
*
* @todo could use performance improvements (it keeps scanning
* the whole message queue for example) and has thread issues,
- * see comments in source
+ * see comments in source.
+ *
+ * @todo holds the dispatch lock right now so blocks other threads
+ * from reading messages. This could be fixed by having
+ * dbus_connection_dispatch() and friends wake up this thread if the
+ * needed reply is seen. That in turn could be done by using
+ * DBusPendingCall for all replies we block for, so we track which
+ * replies are interesting.
*
* Does not re-enter the main loop or run filter/path-registered
* callbacks. The reply to the message will not be seen by
@@ -2253,11 +2264,11 @@
client_serial,
start_tv_sec, start_tv_usec,
end_tv_sec, end_tv_usec);
+
+ _dbus_connection_acquire_dispatch (connection);
/* Now we wait... */
- /* THREAD TODO: This is busted. What if a dispatch() or pop_message
- * gets the message before we do?
- */
+ /* THREAD TODO: Evil to block here and lock all other threads out of dispatch. */
/* always block at least once as we know we don't have the reply yet */
_dbus_connection_do_iteration_unlocked (connection,
DBUS_ITERATION_DO_READING |
@@ -2286,6 +2297,8 @@
_dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n");
+ _dbus_connection_release_dispatch (connection);
+
/* Unlocks, and calls out to user code */
_dbus_connection_update_dispatch_status_and_unlock (connection, status);
@@ -2297,6 +2310,7 @@
if (!_dbus_connection_get_is_connected_unlocked (connection))
{
+ _dbus_connection_release_dispatch (connection);
CONNECTION_UNLOCK (connection);
return NULL;
}
@@ -2342,6 +2356,8 @@
_dbus_verbose ("dbus_connection_send_with_reply_and_block(): Waited %ld milliseconds and got no reply\n",
(tv_sec - start_tv_sec) * 1000 + (tv_usec - start_tv_usec) / 1000);
+ _dbus_connection_release_dispatch (connection);
+
/* unlocks and calls out to user code */
_dbus_connection_update_dispatch_status_and_unlock (connection, status);
@@ -2453,26 +2469,6 @@
_dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
}
-/* 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)
- {
-#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
- }
-}
-
/**
* Returns the first-received message from the incoming message queue,
* leaving it in the queue. If the queue is empty, returns #NULL.
@@ -2484,18 +2480,21 @@
* 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.
*
+ * dbus_connection_dispatch() will block if called while a borrowed
+ * message is outstanding; only one piece of code can be playing with
+ * the incoming queue at a time. This function will block if called
+ * during a dbus_connection_dispatch().
+ *
* @param connection the connection.
* @returns next message in the incoming queue.
*/
DBusMessage*
-dbus_connection_borrow_message (DBusConnection *connection)
+dbus_connection_borrow_message (DBusConnection *connection)
{
- DBusMessage *message;
DBusDispatchStatus status;
+ DBusMessage *message;
_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);
@@ -2508,21 +2507,28 @@
CONNECTION_LOCK (connection);
- if (connection->message_borrowed != NULL)
- _dbus_connection_wait_for_borrowed (connection);
-
- message = _dbus_list_get_first (&connection->incoming_messages);
+ _dbus_connection_acquire_dispatch (connection);
- if (message)
- connection->message_borrowed = message;
+ /* While a message is outstanding, the dispatch lock is held */
+ _dbus_assert (connection->message_borrowed == NULL);
+
+ connection->message_borrowed = _dbus_list_get_first (&connection->incoming_messages);
+ message = connection->message_borrowed;
+
+ /* Note that we KEEP the dispatch lock until the message is returned */
+ if (message == NULL)
+ _dbus_connection_release_dispatch (connection);
+
CONNECTION_UNLOCK (connection);
+
return message;
}
/**
* Used to return a message after peeking at it using
- * dbus_connection_borrow_message().
+ * dbus_connection_borrow_message(). Only called if
+ * message from dbus_connection_borrow_message() was non-#NULL.
*
* @param connection the connection
* @param message the message from dbus_connection_borrow_message()
@@ -2533,15 +2539,16 @@
{
_dbus_return_if_fail (connection != NULL);
_dbus_return_if_fail (message != NULL);
- /* can't borrow during dispatch */
- _dbus_return_if_fail (!connection->dispatch_acquired);
+ _dbus_return_if_fail (message == connection->message_borrowed);
+ _dbus_return_if_fail (connection->dispatch_acquired);
CONNECTION_LOCK (connection);
_dbus_assert (message == connection->message_borrowed);
connection->message_borrowed = NULL;
- dbus_condvar_wake_all (connection->message_returned_cond);
+
+ _dbus_connection_release_dispatch (connection);
CONNECTION_UNLOCK (connection);
}
@@ -2563,8 +2570,8 @@
_dbus_return_if_fail (connection != NULL);
_dbus_return_if_fail (message != NULL);
- /* can't borrow during dispatch */
- _dbus_return_if_fail (!connection->dispatch_acquired);
+ _dbus_return_if_fail (message == connection->message_borrowed);
+ _dbus_return_if_fail (connection->dispatch_acquired);
CONNECTION_LOCK (connection);
@@ -2579,7 +2586,8 @@
message, connection->n_incoming);
connection->message_borrowed = NULL;
- dbus_condvar_wake_all (connection->message_returned_cond);
+
+ _dbus_connection_release_dispatch (connection);
CONNECTION_UNLOCK (connection);
}
@@ -2592,8 +2600,7 @@
{
HAVE_LOCK_CHECK (connection);
- if (connection->message_borrowed != NULL)
- _dbus_connection_wait_for_borrowed (connection);
+ _dbus_assert (connection->message_borrowed == NULL);
if (connection->n_incoming > 0)
{
@@ -2656,6 +2663,8 @@
_dbus_assert (message_link != NULL);
/* You can't borrow a message while a link is outstanding */
_dbus_assert (connection->message_borrowed == NULL);
+ /* We had to have the dispatch lock across the pop/putback */
+ _dbus_assert (connection->dispatch_acquired);
_dbus_list_prepend_link (&connection->incoming_messages,
message_link);
@@ -2685,6 +2694,11 @@
* useful in very simple programs that don't share a #DBusConnection
* with any libraries or other modules.
*
+ * There is a lock that covers all ways of accessing the incoming message
+ * queue, so dbus_connection_dispatch(), dbus_connection_pop_message(),
+ * dbus_connection_borrow_message(), etc. will all block while one of the others
+ * in the group is running.
+ *
* @param connection the connection.
* @returns next message in the incoming queue.
*/
@@ -2704,11 +2718,14 @@
return NULL;
CONNECTION_LOCK (connection);
-
+ _dbus_connection_acquire_dispatch (connection);
+ HAVE_LOCK_CHECK (connection);
+
message = _dbus_connection_pop_message_unlocked (connection);
_dbus_verbose ("Returning popped message %p\n", message);
-
+
+ _dbus_connection_release_dispatch (connection);
CONNECTION_UNLOCK (connection);
return message;
Index: dbus-internals.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-internals.c,v
retrieving revision 1.42
retrieving revision 1.43
diff -u -d -r1.42 -r1.43
--- dbus-internals.c 28 Jan 2005 03:06:55 -0000 1.42
+++ dbus-internals.c 13 Feb 2005 20:21:59 -0000 1.43
@@ -215,6 +215,7 @@
static dbus_bool_t verbose_initted = FALSE;
+#include <pthread.h>
/**
* Prints a warning message to stderr
* if the user has enabled verbose mode.
@@ -249,7 +250,7 @@
/* Print out pid before the line */
if (need_pid)
- fprintf (stderr, "%lu: ", _dbus_getpid ());
+ fprintf (stderr, "%lu: 0x%lx: ", _dbus_getpid (), pthread_self ());
/* Only print pid again if the next line is a new line */
len = strlen (format);
More information about the dbus-commit
mailing list