dbus/dbus dbus-connection-internal.h, 1.23, 1.24 dbus-connection.c,
1.97, 1.98 dbus-pending-call.c, 1.10, 1.11 dbus-pending-call.h,
1.6, 1.7
Havoc Pennington
hp at freedesktop.org
Tue Feb 15 20:37:29 PST 2005
Update of /cvs/dbus/dbus/dbus
In directory gabe:/tmp/cvs-serv23849/dbus
Modified Files:
dbus-connection-internal.h dbus-connection.c
dbus-pending-call.c dbus-pending-call.h
Log Message:
2005-02-15 Havoc Pennington <hp at redhat.com>
* dbus/dbus-connection.c (dbus_connection_dispatch): always
complete a pending call, don't run filters first.
* glib/dbus-gproxy.c (dbus_g_proxy_end_call): change to use
dbus_pending_call_steal_reply
* dbus/dbus-pending-call.c (dbus_pending_call_block): just call
_dbus_connection_block_pending_call
(dbus_pending_call_get_reply): change to steal_reply and return a
ref
* dbus/dbus-connection.c
(dbus_connection_send_with_reply_and_block): port to work in terms
of DBusPendingCall
(_dbus_connection_block_pending_call): replace block_for_reply
with this
Index: dbus-connection-internal.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection-internal.h,v
retrieving revision 1.23
retrieving revision 1.24
diff -u -d -r1.23 -r1.24
--- dbus-connection-internal.h 13 Feb 2005 17:16:25 -0000 1.23
+++ dbus-connection-internal.h 16 Feb 2005 04:37:27 -0000 1.24
@@ -84,9 +84,7 @@
void _dbus_pending_call_notify (DBusPendingCall *pending);
void _dbus_connection_remove_pending_call (DBusConnection *connection,
DBusPendingCall *pending);
-DBusMessage* _dbus_connection_block_for_reply (DBusConnection *connection,
- dbus_uint32_t client_serial,
- int timeout_milliseconds);
+void _dbus_connection_block_pending_call (DBusPendingCall *pending);
void _dbus_pending_call_complete_and_unlock (DBusPendingCall *pending,
DBusMessage *message);
dbus_bool_t _dbus_connection_send_and_unlock (DBusConnection *connection,
Index: dbus-connection.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection.c,v
retrieving revision 1.97
retrieving revision 1.98
diff -u -d -r1.97 -r1.98
--- dbus-connection.c 13 Feb 2005 20:21:59 -0000 1.97
+++ dbus-connection.c 16 Feb 2005 04:37:27 -0000 1.98
@@ -2171,6 +2171,9 @@
return FALSE;
}
+/* This is slightly strange since we can pop a message here without
+ * the dispatch lock.
+ */
static DBusMessage*
check_for_reply_unlocked (DBusConnection *connection,
dbus_uint32_t client_serial)
@@ -2178,9 +2181,6 @@
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);
@@ -2201,53 +2201,49 @@
}
/**
- * Blocks a certain time period while waiting for a reply.
- * If no reply arrives, returns #NULL.
- *
- * @todo could use performance improvements (it keeps scanning
- * the whole message queue for example) and has thread issues,
- * 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.
+ * Blocks until a pending call times out or gets a reply.
*
* Does not re-enter the main loop or run filter/path-registered
* callbacks. The reply to the message will not be seen by
* filter callbacks.
*
- * @param connection the connection
- * @param client_serial the reply serial to wait for
- * @param timeout_milliseconds timeout in milliseconds or -1 for default
- * @returns the message that is the reply or #NULL if no reply
+ * Returns immediately if pending call already got a reply.
+ *
+ * @todo could use performance improvements (it keeps scanning
+ * the whole message queue for example)
+ *
+ * @param pending the pending call we block for a reply on
*/
-DBusMessage*
-_dbus_connection_block_for_reply (DBusConnection *connection,
- dbus_uint32_t client_serial,
- int timeout_milliseconds)
+void
+_dbus_connection_block_pending_call (DBusPendingCall *pending)
{
long start_tv_sec, start_tv_usec;
long end_tv_sec, end_tv_usec;
long tv_sec, tv_usec;
DBusDispatchStatus status;
+ DBusConnection *connection;
+ dbus_uint32_t client_serial;
+ int timeout_milliseconds;
- _dbus_assert (connection != NULL);
- _dbus_assert (client_serial != 0);
- _dbus_assert (timeout_milliseconds >= 0 || timeout_milliseconds == -1);
+ _dbus_assert (pending != NULL);
+
+ if (dbus_pending_call_get_completed (pending))
+ return;
+
+ if (pending->connection == NULL)
+ return; /* call already detached */
+
+ dbus_pending_call_ref (pending); /* necessary because the call could be canceled */
- if (timeout_milliseconds == -1)
- timeout_milliseconds = _DBUS_DEFAULT_TIMEOUT_VALUE;
+ connection = pending->connection;
+ client_serial = pending->reply_serial;
- /* it would probably seem logical to pass in _DBUS_INT_MAX
- * for infinite timeout, but then math below would get
- * all overflow-prone, so smack that down.
+ /* note that timeout_milliseconds is limited to a smallish value
+ * in _dbus_pending_call_new() so overflows aren't possible
+ * below
*/
- if (timeout_milliseconds > _DBUS_ONE_HOUR_IN_MILLISECONDS * 6)
- timeout_milliseconds = _DBUS_ONE_HOUR_IN_MILLISECONDS * 6;
-
+ timeout_milliseconds = dbus_timeout_get_interval (pending->timeout);
+
/* Flush message queue */
dbus_connection_flush (connection);
@@ -2265,10 +2261,7 @@
start_tv_sec, start_tv_usec,
end_tv_sec, end_tv_usec);
- _dbus_connection_acquire_dispatch (connection);
-
/* Now we wait... */
- /* 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 |
@@ -2285,6 +2278,16 @@
status = _dbus_connection_get_dispatch_status_unlocked (connection);
+ /* the get_completed() is in case a dispatch() while we were blocking
+ * got the reply instead of us.
+ */
+ if (dbus_pending_call_get_completed (pending))
+ {
+ _dbus_verbose ("Pending call completed by dispatch in %s\n", _DBUS_FUNCTION_NAME);
+ _dbus_connection_update_dispatch_status_and_unlock (connection, status);
+ return;
+ }
+
if (status == DBUS_DISPATCH_DATA_REMAINS)
{
DBusMessage *reply;
@@ -2293,16 +2296,17 @@
if (reply != NULL)
{
_dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME);
- status = _dbus_connection_get_dispatch_status_unlocked (connection);
_dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n");
-
- _dbus_connection_release_dispatch (connection);
- /* Unlocks, and calls out to user code */
+ _dbus_pending_call_complete_and_unlock (pending, reply);
+ dbus_message_unref (reply);
+
+ CONNECTION_LOCK (connection);
+ status = _dbus_connection_get_dispatch_status_unlocked (connection);
_dbus_connection_update_dispatch_status_and_unlock (connection, status);
- return reply;
+ return;
}
}
@@ -2310,9 +2314,13 @@
if (!_dbus_connection_get_is_connected_unlocked (connection))
{
- _dbus_connection_release_dispatch (connection);
- CONNECTION_UNLOCK (connection);
- return NULL;
+ /* FIXME send a "DBUS_ERROR_DISCONNECTED" instead, just to help
+ * programmers understand what went wrong since the timeout is
+ * confusing
+ */
+
+ _dbus_pending_call_complete_and_unlock (pending, NULL);
+ return;
}
else if (tv_sec < start_tv_sec)
_dbus_verbose ("dbus_connection_send_with_reply_and_block(): clock set backward\n");
@@ -2356,12 +2364,15 @@
_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);
+ _dbus_assert (!dbus_pending_call_get_completed (pending));
- /* unlocks and calls out to user code */
- _dbus_connection_update_dispatch_status_and_unlock (connection, status);
+ /* unlock and call user code */
+ _dbus_pending_call_complete_and_unlock (pending, NULL);
- return NULL;
+ /* update user code on dispatch status */
+ CONNECTION_LOCK (connection);
+ status = _dbus_connection_get_dispatch_status_unlocked (connection);
+ _dbus_connection_update_dispatch_status_and_unlock (connection, status);
}
/**
@@ -2386,40 +2397,40 @@
* @returns the message that is the reply or #NULL with an error code if the
* function fails.
*/
-DBusMessage *
+DBusMessage*
dbus_connection_send_with_reply_and_block (DBusConnection *connection,
DBusMessage *message,
int timeout_milliseconds,
DBusError *error)
{
- dbus_uint32_t client_serial;
DBusMessage *reply;
+ DBusPendingCall *pending;
_dbus_return_val_if_fail (connection != NULL, NULL);
_dbus_return_val_if_fail (message != NULL, NULL);
_dbus_return_val_if_fail (timeout_milliseconds >= 0 || timeout_milliseconds == -1, FALSE);
_dbus_return_val_if_error_is_set (error, NULL);
- if (!dbus_connection_send (connection, message, &client_serial))
+ if (!dbus_connection_send_with_reply (connection, message,
+ &pending, timeout_milliseconds))
{
_DBUS_SET_OOM (error);
return NULL;
}
- reply = _dbus_connection_block_for_reply (connection,
- client_serial,
- timeout_milliseconds);
+ _dbus_assert (pending != NULL);
- if (reply == NULL)
- {
- if (dbus_connection_get_is_connected (connection))
- dbus_set_error (error, DBUS_ERROR_NO_REPLY, "Message did not receive a reply");
- else
- dbus_set_error (error, DBUS_ERROR_DISCONNECTED, "Disconnected prior to receiving a reply");
+ dbus_pending_call_block (pending);
- return NULL;
- }
- else if (dbus_set_error_from_message (error, reply))
+ reply = dbus_pending_call_steal_reply (pending);
+ dbus_pending_call_unref (pending);
+
+ /* call_complete_and_unlock() called from pending_call_block() should
+ * always fill this in.
+ */
+ _dbus_assert (reply != NULL);
+
+ if (dbus_set_error_from_message (error, reply))
{
dbus_message_unref (reply);
return NULL;
@@ -2934,16 +2945,12 @@
*
* @todo some FIXME in here about handling DBUS_HANDLER_RESULT_NEED_MEMORY
*
- * @todo right now a message filter gets run on replies to a pending
- * call in here, but not in the case where we block without entering
- * the main loop. Simple solution might be to just have the pending
- * call stuff run before the filters.
- *
* @todo FIXME what if we call out to application code to handle a
* message, holding the dispatch lock, and the application code runs
* the main loop and dispatches again? Probably deadlocks at the
* moment. Maybe we want a dispatch status of DBUS_DISPATCH_IN_PROGRESS,
- * and then the GSource etc. could handle the situation?
+ * and then the GSource etc. could handle the situation? Right now
+ * our GSource is NO_RECURSE
*
* @param connection the connection
* @returns dispatch status
@@ -2979,18 +2986,12 @@
_dbus_connection_acquire_dispatch (connection);
HAVE_LOCK_CHECK (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() is still
- * protected by the lock, since only one will get the lock, and that
- * one will finish the message dispatching
- */
message_link = _dbus_connection_pop_message_link_unlocked (connection);
if (message_link == NULL)
{
/* another thread dispatched our stuff */
- _dbus_verbose ("another thread dispatched message\n");
+ _dbus_verbose ("another thread dispatched message (during acquire_dispatch above)\n");
_dbus_connection_release_dispatch (connection);
@@ -3015,24 +3016,44 @@
dbus_message_get_member (message) :
"no member",
dbus_message_get_signature (message));
-
- result = DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+ result = DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+
+ /* Pending call handling must be first, because if you do
+ * dbus_connection_send_with_reply_and_block() or
+ * dbus_pending_call_block() then no handlers/filters will be run on
+ * the reply. We want consistent semantics in the case where we
+ * dbus_connection_dispatch() the reply.
+ */
+
reply_serial = dbus_message_get_reply_serial (message);
pending = _dbus_hash_table_lookup_int (connection->pending_replies,
reply_serial);
+ if (pending)
+ {
+ _dbus_verbose ("Dispatching a pending reply\n");
+ _dbus_pending_call_complete_and_unlock (pending, message);
+ pending = NULL; /* it's probably unref'd */
+
+ CONNECTION_LOCK (connection);
+ _dbus_verbose ("pending call completed in dispatch\n");
+ result = DBUS_HANDLER_RESULT_HANDLED;
+ goto out;
+ }
if (!_dbus_list_copy (&connection->filter_list, &filter_list_copy))
{
_dbus_connection_release_dispatch (connection);
HAVE_LOCK_CHECK (connection);
-
+
_dbus_connection_failed_pop (connection, message_link);
/* unlocks and calls user code */
_dbus_connection_update_dispatch_status_and_unlock (connection,
DBUS_DISPATCH_NEED_MEMORY);
+ if (pending)
+ dbus_pending_call_unref (pending);
dbus_connection_unref (connection);
return DBUS_DISPATCH_NEED_MEMORY;
@@ -3074,40 +3095,11 @@
_dbus_verbose ("No memory in %s\n", _DBUS_FUNCTION_NAME);
goto out;
}
-
- /* Did a reply we were waiting on get filtered? */
- if (pending && result == DBUS_HANDLER_RESULT_HANDLED)
- {
- /* Queue the timeout immediately! */
- if (pending->timeout_link)
- {
- _dbus_connection_queue_synthesized_message_link (connection,
- pending->timeout_link);
- pending->timeout_link = NULL;
- }
- else
- {
- /* We already queued the timeout? Then it was filtered! */
- _dbus_warn ("The timeout error with reply serial %d was filtered, so the DBusPendingCall will never stop pending.\n", reply_serial);
- }
- }
-
- if (result == DBUS_HANDLER_RESULT_HANDLED)
+ else if (result == DBUS_HANDLER_RESULT_HANDLED)
{
_dbus_verbose ("filter handled message in dispatch\n");
goto out;
}
-
- if (pending)
- {
- _dbus_pending_call_complete_and_unlock (pending, message);
-
- pending = NULL;
-
- CONNECTION_LOCK (connection);
- _dbus_verbose ("pending call completed in dispatch\n");
- goto out;
- }
/* We're still protected from dispatch() reentrancy here
* since we acquired the dispatcher
Index: dbus-pending-call.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-pending-call.c,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -d -r1.10 -r1.11
--- dbus-pending-call.c 30 Jan 2005 23:06:32 -0000 1.10
+++ dbus-pending-call.c 16 Feb 2005 04:37:27 -0000 1.11
@@ -61,6 +61,14 @@
if (timeout_milliseconds == -1)
timeout_milliseconds = _DBUS_DEFAULT_TIMEOUT_VALUE;
+ /* it would probably seem logical to pass in _DBUS_INT_MAX for
+ * infinite timeout, but then math in
+ * _dbus_connection_block_for_reply would get all overflow-prone, so
+ * smack that down.
+ */
+ if (timeout_milliseconds > _DBUS_ONE_HOUR_IN_MILLISECONDS * 6)
+ timeout_milliseconds = _DBUS_ONE_HOUR_IN_MILLISECONDS * 6;
+
if (!dbus_pending_call_allocate_data_slot (¬ify_user_data_slot))
return NULL;
@@ -102,6 +110,8 @@
void
_dbus_pending_call_notify (DBusPendingCall *pending)
{
+ _dbus_assert (!pending->completed);
+
pending->completed = TRUE;
if (pending->function)
@@ -258,21 +268,26 @@
}
/**
- * Gets the reply, or returns #NULL if none has been received yet. The
- * reference count is not incremented on the returned message, so you
- * have to keep a reference count on the pending call (or add one
- * to the message).
- *
- * @todo not thread safe? I guess it has to lock though it sucks
- * @todo maybe to make this threadsafe, it should be steal_reply(), i.e. only one thread can ever get the message
+ * Gets the reply, or returns #NULL if none has been received
+ * yet. Ownership of the reply message passes to the caller. This
+ * function can only be called once per pending call, since the reply
+ * message is tranferred to the caller.
*
* @param pending the pending call
* @returns the reply message or #NULL.
*/
DBusMessage*
-dbus_pending_call_get_reply (DBusPendingCall *pending)
+dbus_pending_call_steal_reply (DBusPendingCall *pending)
{
- return pending->reply;
+ DBusMessage *message;
+
+ _dbus_return_val_if_fail (pending->completed, NULL);
+ _dbus_return_val_if_fail (pending->reply != NULL, NULL);
+
+ message = pending->reply;
+ pending->reply = NULL;
+
+ return message;
}
/**
@@ -292,20 +307,7 @@
void
dbus_pending_call_block (DBusPendingCall *pending)
{
- DBusMessage *message;
-
- if (dbus_pending_call_get_completed (pending))
- return;
-
- /* message may be NULL if no reply */
- message = _dbus_connection_block_for_reply (pending->connection,
- pending->reply_serial,
- dbus_timeout_get_interval (pending->timeout));
-
- _dbus_connection_lock (pending->connection);
- _dbus_pending_call_complete_and_unlock (pending, message);
- if (message)
- dbus_message_unref (message);
+ _dbus_connection_block_pending_call (pending);
}
static DBusDataSlotAllocator slot_allocator;
Index: dbus-pending-call.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-pending-call.h,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -d -r1.6 -r1.7
--- dbus-pending-call.h 9 Sep 2004 10:20:17 -0000 1.6
+++ dbus-pending-call.h 16 Feb 2005 04:37:27 -0000 1.7
@@ -41,7 +41,7 @@
DBusFreeFunction free_user_data);
void dbus_pending_call_cancel (DBusPendingCall *pending);
dbus_bool_t dbus_pending_call_get_completed (DBusPendingCall *pending);
-DBusMessage* dbus_pending_call_get_reply (DBusPendingCall *pending);
+DBusMessage* dbus_pending_call_steal_reply (DBusPendingCall *pending);
void dbus_pending_call_block (DBusPendingCall *pending);
dbus_bool_t dbus_pending_call_allocate_data_slot (dbus_int32_t *slot_p);
More information about the dbus-commit
mailing list