Commit 9e445087 authored by Havoc Pennington's avatar Havoc Pennington
Browse files

2005-02-15 Havoc Pennington <hp@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
parent 7b340cdb
2005-02-15 Havoc Pennington <hp@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
2005-02-14 Havoc Pennington <hp@redhat.com>
* dbus/dbus-userdb-util.c (_dbus_user_database_lookup_group):
......
......@@ -84,9 +84,7 @@ DBusPendingCall* _dbus_pending_call_new (DBusConnection
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,
......
......@@ -2171,6 +2171,9 @@ dbus_connection_send_with_reply (DBusConnection *connection,
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 @@ check_for_reply_unlocked (DBusConnection *connection,
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 @@ check_for_reply_unlocked (DBusConnection *connection,
}
/**
* 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 @@ _dbus_connection_block_for_reply (DBusConnection *connection,
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 @@ _dbus_connection_block_for_reply (DBusConnection *connection,
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 @@ _dbus_connection_block_for_reply (DBusConnection *connection,
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 @@ _dbus_connection_block_for_reply (DBusConnection *connection,
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_connection_block_for_reply (DBusConnection *connection,
_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 @@ _dbus_connection_block_for_reply (DBusConnection *connection,
* @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 @@ dbus_connection_get_dispatch_status (DBusConnection *connection)
*
* @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_dispatch (DBusConnection *connection)
_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_connection_dispatch (DBusConnection *connection)
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_connection_dispatch (DBusConnection *connection)
_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
......
......@@ -61,6 +61,14 @@ _dbus_pending_call_new (DBusConnection *connection,
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 (&notify_user_data_slot))
return NULL;
......@@ -102,6 +110,8 @@ _dbus_pending_call_new (DBusConnection *connection,
void
_dbus_pending_call_notify (DBusPendingCall *pending)
{
_dbus_assert (!pending->completed);
pending->completed = TRUE;
if (pending->function)
......@@ -258,21 +268,26 @@ dbus_pending_call_get_completed (DBusPendingCall *pending)
}
/**
* 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 @@ dbus_pending_call_get_reply (DBusPendingCall *pending)
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;
......
......@@ -41,7 +41,7 @@ dbus_bool_t dbus_pending_call_set_notify (DBusPendingCall *pen
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);
......
......@@ -33,6 +33,8 @@ Important for 1.0
- make the mutex/condvar functions private
- dbus-pending-call.c has some API and thread safety issues to review
Important for 1.0 GLib Bindings
===
......
......@@ -1386,7 +1386,7 @@ dbus_g_proxy_end_call (DBusGProxy *proxy,
g_return_val_if_fail (pending != NULL, FALSE);
dbus_pending_call_block (DBUS_PENDING_CALL_FROM_G_PENDING_CALL (pending));
message = dbus_pending_call_get_reply (DBUS_PENDING_CALL_FROM_G_PENDING_CALL (pending));
message = dbus_pending_call_steal_reply (DBUS_PENDING_CALL_FROM_G_PENDING_CALL (pending));
g_assert (message != NULL);
......@@ -1403,6 +1403,7 @@ dbus_g_proxy_end_call (DBusGProxy *proxy,
}
va_end (args);
dbus_message_unref (message);
return TRUE;
case DBUS_MESSAGE_TYPE_ERROR:
......@@ -1416,6 +1417,8 @@ dbus_g_proxy_end_call (DBusGProxy *proxy,
}
error:
dbus_message_unref (message);
dbus_set_g_error (error, &derror);
dbus_error_free (&derror);
return FALSE;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment