From 222bd07e9df5e3b5a367d1282b43fd3a827a7552 Mon Sep 17 00:00:00 2001 From: "John (J5) Palmieri" Date: Fri, 4 Aug 2006 16:15:16 +0000 Subject: * configure.in: add -Wdeclaration-after-statement * dbus/dbus-connection.c: change all the pending call stuff to reflect the fact that pending call operations use the connection lock * dbus/dbus-pending-call.c: add locking here * dbus/dbus-errors.c (struct DBusRealError): don't make the name field const consistent with how message field is done --- ChangeLog | 13 ++ configure.in | 5 + dbus/dbus-connection.c | 229 +++++++++++++++++------------- dbus/dbus-errors.c | 4 +- dbus/dbus-pending-call-internal.h | 53 +++---- dbus/dbus-pending-call.c | 292 +++++++++++++++++++++++++++++--------- 6 files changed, 405 insertions(+), 191 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0d81e25f..9d58a6a3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2006-08-04 Havoc Pennington + + * configure.in: add -Wdeclaration-after-statement + + * dbus/dbus-connection.c: change all the pending call stuff to + reflect the fact that pending call operations use the connection + lock + + * dbus/dbus-pending-call.c: add locking here + + * dbus/dbus-errors.c (struct DBusRealError): don't make the name + field const consistent with how message field is done + 2006-08-03 John (J5) Palmieri * s/D-BUS/D-Bus/g diff --git a/configure.in b/configure.in index 65736a89..dfad86ed 100644 --- a/configure.in +++ b/configure.in @@ -138,6 +138,11 @@ if test "x$GCC" = "xyes"; then *) CFLAGS="$CFLAGS -Wsign-compare" ;; esac + case " $CFLAGS " in + *[\ \ ]-Wdeclaration-after-statement[\ \ ]*) ;; + *) CFLAGS="$CFLAGS -Wdeclaration-after-statement" ;; + esac + if test "x$enable_ansi" = "xyes"; then case " $CFLAGS " in *[\ \ ]-ansi[\ \ ]*) ;; diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 73888d41..2e942064 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -260,6 +260,7 @@ static void _dbus_connection_update_dispatch_status_and_unlock (DB 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 DBusDispatchStatus _dbus_connection_flush_unlocked (DBusConnection *connection); static DBusMessageFilter * _dbus_message_filter_ref (DBusMessageFilter *filter) @@ -378,11 +379,11 @@ _dbus_connection_queue_received_message_link (DBusConnection *connection, reply_serial); if (pending != NULL) { - if (_dbus_pending_call_is_timeout_added (pending)) + if (_dbus_pending_call_is_timeout_added_unlocked (pending)) _dbus_connection_remove_timeout_unlocked (connection, - _dbus_pending_call_get_timeout (pending)); + _dbus_pending_call_get_timeout_unlocked (pending)); - _dbus_pending_call_set_timeout_added (pending, FALSE); + _dbus_pending_call_set_timeout_added_unlocked (pending, FALSE); } } @@ -783,11 +784,11 @@ _dbus_connection_attach_pending_call_unlocked (DBusConnection *connection, HAVE_LOCK_CHECK (connection); - reply_serial = _dbus_pending_call_get_reply_serial (pending); + reply_serial = _dbus_pending_call_get_reply_serial_unlocked (pending); _dbus_assert (reply_serial != 0); - timeout = _dbus_pending_call_get_timeout (pending); + timeout = _dbus_pending_call_get_timeout_unlocked (pending); if (!_dbus_connection_add_timeout_unlocked (connection, timeout)) return FALSE; @@ -798,14 +799,14 @@ _dbus_connection_attach_pending_call_unlocked (DBusConnection *connection, { _dbus_connection_remove_timeout_unlocked (connection, timeout); - _dbus_pending_call_set_timeout_added (pending, FALSE); + _dbus_pending_call_set_timeout_added_unlocked (pending, FALSE); HAVE_LOCK_CHECK (connection); return FALSE; } - _dbus_pending_call_set_timeout_added (pending, TRUE); + _dbus_pending_call_set_timeout_added_unlocked (pending, TRUE); - dbus_pending_call_ref (pending); + _dbus_pending_call_ref_unlocked (pending); HAVE_LOCK_CHECK (connection); @@ -816,39 +817,44 @@ static void free_pending_call_on_hash_removal (void *data) { DBusPendingCall *pending; - DBusConnection *connection; - + DBusConnection *connection; + if (data == NULL) return; pending = data; - connection = _dbus_pending_call_get_connection (pending); + connection = _dbus_pending_call_get_connection_unlocked (pending); - if (connection) + HAVE_LOCK_CHECK (connection); + + if (_dbus_pending_call_is_timeout_added_unlocked (pending)) { - if (_dbus_pending_call_is_timeout_added (pending)) - { - _dbus_connection_remove_timeout_unlocked (connection, - _dbus_pending_call_get_timeout (pending)); + _dbus_connection_remove_timeout_unlocked (connection, + _dbus_pending_call_get_timeout_unlocked (pending)); - _dbus_pending_call_set_timeout_added (pending, FALSE); - } - - dbus_pending_call_unref (pending); + _dbus_pending_call_set_timeout_added_unlocked (pending, FALSE); } + + /* FIXME this is sort of dangerous and undesirable to drop the lock here, but + * the pending call finalizer could in principle call out to application code + * so we pretty much have to... some larger code reorg might be needed. + */ + _dbus_connection_ref_unlocked (connection); + _dbus_pending_call_unref_and_unlock (pending); + CONNECTION_LOCK (connection); + _dbus_connection_unref_unlocked (connection); } static void _dbus_connection_detach_pending_call_unlocked (DBusConnection *connection, DBusPendingCall *pending) { - /* Can't have a destroy notifier on the pending call if we're going to do this */ - - dbus_pending_call_ref (pending); + /* This ends up unlocking to call the pending call finalizer, which is unexpected to + * say the least. + */ _dbus_hash_table_remove_int (connection->pending_replies, - _dbus_pending_call_get_reply_serial (pending)); - dbus_pending_call_unref (pending); + _dbus_pending_call_get_reply_serial_unlocked (pending)); } static void @@ -858,12 +864,14 @@ _dbus_connection_detach_pending_call_and_unlock (DBusConnection *connection, /* The idea here is to avoid finalizing the pending call * with the lock held, since there's a destroy notifier * in pending call that goes out to application code. + * + * There's an extra unlock inside the hash table + * "free pending call" function FIXME... */ - dbus_pending_call_ref (pending); + _dbus_pending_call_ref_unlocked (pending); _dbus_hash_table_remove_int (connection->pending_replies, - _dbus_pending_call_get_reply_serial (pending)); - CONNECTION_UNLOCK (connection); - dbus_pending_call_unref (pending); + _dbus_pending_call_get_reply_serial_unlocked (pending)); + _dbus_pending_call_unref_and_unlock (pending); } /** @@ -2311,14 +2319,13 @@ reply_handler_timeout (void *data) DBusDispatchStatus status; DBusPendingCall *pending = data; - connection = _dbus_pending_call_get_connection (pending); - - CONNECTION_LOCK (connection); - _dbus_pending_call_queue_timeout_error (pending, - connection); + connection = _dbus_pending_call_get_connection_and_lock (pending); + + _dbus_pending_call_queue_timeout_error_unlocked (pending, + connection); _dbus_connection_remove_timeout_unlocked (connection, - _dbus_pending_call_get_timeout (pending)); - _dbus_pending_call_set_timeout_added (pending, FALSE); + _dbus_pending_call_get_timeout_unlocked (pending)); + _dbus_pending_call_set_timeout_added_unlocked (pending, FALSE); _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); status = _dbus_connection_get_dispatch_status_unlocked (connection); @@ -2394,9 +2401,9 @@ dbus_connection_send_with_reply (DBusConnection *connection, return TRUE; } - pending = _dbus_pending_call_new (connection, - timeout_milliseconds, - reply_handler_timeout); + pending = _dbus_pending_call_new_unlocked (connection, + timeout_milliseconds, + reply_handler_timeout); if (pending == NULL) { @@ -2412,7 +2419,7 @@ dbus_connection_send_with_reply (DBusConnection *connection, _dbus_message_set_serial (message, serial); } - if (!_dbus_pending_call_set_timeout_error (pending, message, serial)) + if (!_dbus_pending_call_set_timeout_error_unlocked (pending, message, serial)) goto error; /* Insert the serial in the pending replies hash; @@ -2431,19 +2438,23 @@ dbus_connection_send_with_reply (DBusConnection *connection, } if (pending_return) - *pending_return = pending; + *pending_return = pending; /* hand off refcount */ else { _dbus_connection_detach_pending_call_unlocked (connection, pending); - dbus_pending_call_unref (pending); + /* we still have a ref to the pending call in this case, we unref + * after unlocking, below + */ } - _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); status = _dbus_connection_get_dispatch_status_unlocked (connection); /* this calls out to user code */ _dbus_connection_update_dispatch_status_and_unlock (connection, status); + if (pending_return == NULL) + dbus_pending_call_unref (pending); + return TRUE; error: @@ -2485,62 +2496,65 @@ check_for_reply_unlocked (DBusConnection *connection, static void connection_timeout_and_complete_all_pending_calls_unlocked (DBusConnection *connection) { - DBusHashIter iter; - - _dbus_hash_iter_init (connection->pending_replies, &iter); - - /* create list while we remove the iters from the hash - because we need to go over it a couple of times */ - while (_dbus_hash_iter_next (&iter)) + /* We can't iterate over the hash in the normal way since we'll be + * dropping the lock for each item. So we restart the + * iter each time as we drain the hash table. + */ + + while (_dbus_hash_table_get_n_entries (connection->pending_replies) > 0) { DBusPendingCall *pending; - + DBusHashIter iter; + + _dbus_hash_iter_init (connection->pending_replies, &iter); + _dbus_hash_iter_next (&iter); + pending = (DBusPendingCall *) _dbus_hash_iter_get_value (&iter); - dbus_pending_call_ref (pending); - - _dbus_pending_call_queue_timeout_error (pending, - connection); + _dbus_pending_call_ref_unlocked (pending); + + _dbus_pending_call_queue_timeout_error_unlocked (pending, + connection); _dbus_connection_remove_timeout_unlocked (connection, - _dbus_pending_call_get_timeout (pending)); - - _dbus_pending_call_set_timeout_added (pending, FALSE); + _dbus_pending_call_get_timeout_unlocked (pending)); + _dbus_pending_call_set_timeout_added_unlocked (pending, FALSE); _dbus_hash_iter_remove_entry (&iter); - dbus_pending_call_unref (pending); + _dbus_pending_call_unref_and_unlock (pending); + CONNECTION_LOCK (connection); } + HAVE_LOCK_CHECK (connection); } static void -complete_pending_call_and_unlock (DBusPendingCall *pending, +complete_pending_call_and_unlock (DBusConnection *connection, + DBusPendingCall *pending, DBusMessage *message) { - _dbus_pending_call_set_reply (pending, message); - dbus_pending_call_ref (pending); /* in case there's no app with a ref held */ - _dbus_connection_detach_pending_call_and_unlock (_dbus_pending_call_get_connection (pending), pending); - + _dbus_pending_call_set_reply_unlocked (pending, message); + _dbus_pending_call_ref_unlocked (pending); /* in case there's no app with a ref held */ + _dbus_connection_detach_pending_call_and_unlock (connection, pending); + /* Must be called unlocked since it invokes app callback */ _dbus_pending_call_complete (pending); dbus_pending_call_unref (pending); } static dbus_bool_t -check_for_reply_and_update_dispatch_unlocked (DBusPendingCall *pending) +check_for_reply_and_update_dispatch_unlocked (DBusConnection *connection, + DBusPendingCall *pending) { DBusMessage *reply; DBusDispatchStatus status; - DBusConnection *connection; - - connection = _dbus_pending_call_get_connection (pending); reply = check_for_reply_unlocked (connection, - _dbus_pending_call_get_reply_serial (pending)); + _dbus_pending_call_get_reply_serial_unlocked (pending)); if (reply != NULL) { _dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME); _dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n"); - complete_pending_call_and_unlock (pending, reply); + complete_pending_call_and_unlock (connection, pending, reply); dbus_message_unref (reply); CONNECTION_LOCK (connection); @@ -2606,24 +2620,21 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) if (dbus_pending_call_get_completed (pending)) return; - connection = _dbus_pending_call_get_connection (pending); - if (connection == NULL) - return; /* call already detached */ - dbus_pending_call_ref (pending); /* necessary because the call could be canceled */ - client_serial = _dbus_pending_call_get_reply_serial (pending); + + connection = _dbus_pending_call_get_connection_and_lock (pending); + + /* Flush message queue - note, can affect dispatch status */ + _dbus_connection_flush_unlocked (connection); + + client_serial = _dbus_pending_call_get_reply_serial_unlocked (pending); /* note that timeout_milliseconds is limited to a smallish value * in _dbus_pending_call_new() so overflows aren't possible * below */ - timeout_milliseconds = dbus_timeout_get_interval (_dbus_pending_call_get_timeout (pending)); - - /* Flush message queue */ - dbus_connection_flush (connection); - - CONNECTION_LOCK (connection); - + timeout_milliseconds = dbus_timeout_get_interval (_dbus_pending_call_get_timeout_unlocked (pending)); + _dbus_get_current_time (&start_tv_sec, &start_tv_usec); end_tv_sec = start_tv_sec + timeout_milliseconds / 1000; end_tv_usec = start_tv_usec + (timeout_milliseconds % 1000) * 1000; @@ -2638,7 +2649,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) /* check to see if we already got the data off the socket */ /* from another blocked pending call */ - if (check_for_reply_and_update_dispatch_unlocked (pending)) + if (check_for_reply_and_update_dispatch_unlocked (connection, pending)) return; /* Now we wait... */ @@ -2661,7 +2672,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) /* 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)) + if (_dbus_pending_call_get_completed_unlocked (pending)) { _dbus_verbose ("Pending call completed by dispatch in %s\n", _DBUS_FUNCTION_NAME); _dbus_connection_update_dispatch_status_and_unlock (connection, status); @@ -2669,9 +2680,10 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) return; } - if (status == DBUS_DISPATCH_DATA_REMAINS) - if (check_for_reply_and_update_dispatch_unlocked (pending)) - return; + if (status == DBUS_DISPATCH_DATA_REMAINS) { + if (check_for_reply_and_update_dispatch_unlocked (connection, pending)) + return; + } _dbus_get_current_time (&tv_sec, &tv_usec); @@ -2682,7 +2694,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) * confusing */ - complete_pending_call_and_unlock (pending, NULL); + complete_pending_call_and_unlock (connection, pending, NULL); dbus_pending_call_unref (pending); return; } @@ -2723,10 +2735,10 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) _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_assert (!dbus_pending_call_get_completed (pending)); + _dbus_assert (!_dbus_pending_call_get_completed_unlocked (pending)); /* unlock and call user code */ - complete_pending_call_and_unlock (pending, NULL); + complete_pending_call_and_unlock (connection, pending, NULL); /* update user code on dispatch status */ CONNECTION_LOCK (connection); @@ -2801,11 +2813,14 @@ dbus_connection_send_with_reply_and_block (DBusConnection *connection, /** * Blocks until the outgoing message queue is empty. + * Assumes connection lock already held. * + * If you call this, you MUST call update_dispatch_status afterword... + * * @param connection the connection. */ -void -dbus_connection_flush (DBusConnection *connection) +DBusDispatchStatus +_dbus_connection_flush_unlocked (DBusConnection *connection) { /* We have to specify DBUS_ITERATION_DO_READING here because * otherwise we could have two apps deadlock if they are both doing @@ -2814,9 +2829,8 @@ dbus_connection_flush (DBusConnection *connection) */ DBusDispatchStatus status; - _dbus_return_if_fail (connection != NULL); + HAVE_LOCK_CHECK (connection); - CONNECTION_LOCK (connection); while (connection->n_outgoing > 0 && _dbus_connection_get_is_connected_unlocked (connection)) { @@ -2833,6 +2847,31 @@ dbus_connection_flush (DBusConnection *connection) _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); status = _dbus_connection_get_dispatch_status_unlocked (connection); + HAVE_LOCK_CHECK (connection); + return status; +} + +/** + * Blocks until the outgoing message queue is empty. + * + * @param connection the connection. + */ +void +dbus_connection_flush (DBusConnection *connection) +{ + /* We have to specify DBUS_ITERATION_DO_READING here because + * otherwise we could have two apps deadlock if they are both doing + * a flush(), and the kernel buffers fill up. This could change the + * dispatch status. + */ + DBusDispatchStatus status; + + _dbus_return_if_fail (connection != NULL); + + CONNECTION_LOCK (connection); + + status = _dbus_connection_flush_unlocked (connection); + HAVE_LOCK_CHECK (connection); /* Unlocks and calls out to user code */ _dbus_connection_update_dispatch_status_and_unlock (connection, status); @@ -3590,7 +3629,7 @@ dbus_connection_dispatch (DBusConnection *connection) if (pending) { _dbus_verbose ("Dispatching a pending reply\n"); - complete_pending_call_and_unlock (pending, message); + complete_pending_call_and_unlock (connection, pending, message); pending = NULL; /* it's probably unref'd */ CONNECTION_LOCK (connection); diff --git a/dbus/dbus-errors.c b/dbus/dbus-errors.c index d8f8aa60..fb230852 100644 --- a/dbus/dbus-errors.c +++ b/dbus/dbus-errors.c @@ -40,7 +40,7 @@ */ typedef struct { - const char *name; /**< error name */ + char *name; /**< error name */ char *message; /**< error message */ unsigned int const_message : 1; /**< Message is not owned by DBusError */ @@ -219,7 +219,7 @@ dbus_set_error_const (DBusError *error, real = (DBusRealError *)error; - real->name = name; + real->name = (char*) name; real->message = (char *)message; real->const_message = TRUE; } diff --git a/dbus/dbus-pending-call-internal.h b/dbus/dbus-pending-call-internal.h index 0a5aa25c..03fbdb46 100644 --- a/dbus/dbus-pending-call-internal.h +++ b/dbus/dbus-pending-call-internal.h @@ -31,30 +31,35 @@ DBUS_BEGIN_DECLS -dbus_bool_t _dbus_pending_call_is_timeout_added (DBusPendingCall *pending); -void _dbus_pending_call_set_timeout_added (DBusPendingCall *pending, - dbus_bool_t is_added); -DBusTimeout *_dbus_pending_call_get_timeout (DBusPendingCall *pending); -dbus_uint32_t _dbus_pending_call_get_reply_serial (DBusPendingCall *pending); -void _dbus_pending_call_set_reply_serial (DBusPendingCall *pending, - dbus_uint32_t serial); -DBusConnection *_dbus_pending_call_get_connection (DBusPendingCall *pending); - -void _dbus_pending_call_complete (DBusPendingCall *pending); -void _dbus_pending_call_set_reply (DBusPendingCall *pending, - DBusMessage *message); -void _dbus_pending_call_clear_connection (DBusPendingCall *pending); - -void _dbus_pending_call_queue_timeout_error (DBusPendingCall *pending, - DBusConnection *connection); -void _dbus_pending_call_set_reply_serial (DBusPendingCall *pending, - dbus_uint32_t serial); -dbus_bool_t _dbus_pending_call_set_timeout_error (DBusPendingCall *pending, - DBusMessage *message, - dbus_uint32_t serial); -DBusPendingCall* _dbus_pending_call_new (DBusConnection *connection, - int timeout_milliseconds, - DBusTimeoutHandler timeout_handler); +dbus_bool_t _dbus_pending_call_is_timeout_added_unlocked (DBusPendingCall *pending); +void _dbus_pending_call_set_timeout_added_unlocked (DBusPendingCall *pending, + dbus_bool_t is_added); +DBusTimeout * _dbus_pending_call_get_timeout_unlocked (DBusPendingCall *pending); +dbus_uint32_t _dbus_pending_call_get_reply_serial_unlocked (DBusPendingCall *pending); +void _dbus_pending_call_set_reply_serial_unlocked (DBusPendingCall *pending, + dbus_uint32_t serial); +DBusConnection * _dbus_pending_call_get_connection_and_lock (DBusPendingCall *pending); +DBusConnection * _dbus_pending_call_get_connection_unlocked (DBusPendingCall *pending); +dbus_bool_t _dbus_pending_call_get_completed_unlocked (DBusPendingCall *pending); +void _dbus_pending_call_complete (DBusPendingCall *pending); +void _dbus_pending_call_set_reply_unlocked (DBusPendingCall *pending, + DBusMessage *message); +void _dbus_pending_call_queue_timeout_error_unlocked (DBusPendingCall *pending, + DBusConnection *connection); +void _dbus_pending_call_set_reply_serial_unlocked (DBusPendingCall *pending, + dbus_uint32_t serial); +dbus_bool_t _dbus_pending_call_set_timeout_error_unlocked (DBusPendingCall *pending, + DBusMessage *message, + dbus_uint32_t serial); +DBusPendingCall* _dbus_pending_call_new_unlocked (DBusConnection *connection, + int timeout_milliseconds, + DBusTimeoutHandler timeout_handler); +DBusPendingCall* _dbus_pending_call_ref_unlocked (DBusPendingCall *pending); +void _dbus_pending_call_unref_and_unlock (DBusPendingCall *pending); +dbus_bool_t _dbus_pending_call_set_data_unlocked (DBusPendingCall *pending, + dbus_int32_t slot, + void *data, + DBusFreeFunction free_data_func); DBUS_END_DECLS diff --git a/dbus/dbus-pending-call.c b/dbus/dbus-pending-call.c index da8a3100..e6ece9dd 100644 --- a/dbus/dbus-pending-call.c +++ b/dbus/dbus-pending-call.c @@ -44,6 +44,9 @@ * * Opaque object representing a reply message that we're waiting for. */ +#define CONNECTION_LOCK(connection) _dbus_connection_lock(connection) +#define CONNECTION_UNLOCK(connection) _dbus_connection_unlock(connection) + struct DBusPendingCall { DBusAtomic refcount; /**< reference count */ @@ -75,9 +78,9 @@ static dbus_int32_t notify_user_data_slot = -1; * @returns a new #DBusPendingCall or #NULL if no memory. */ DBusPendingCall* -_dbus_pending_call_new (DBusConnection *connection, - int timeout_milliseconds, - DBusTimeoutHandler timeout_handler) +_dbus_pending_call_new_unlocked (DBusConnection *connection, + int timeout_milliseconds, + DBusTimeoutHandler timeout_handler) { DBusPendingCall *pending; DBusTimeout *timeout; @@ -138,8 +141,8 @@ _dbus_pending_call_new (DBusConnection *connection, * to time out the call */ void -_dbus_pending_call_set_reply (DBusPendingCall *pending, - DBusMessage *message) +_dbus_pending_call_set_reply_unlocked (DBusPendingCall *pending, + DBusMessage *message) { if (message == NULL) { @@ -187,9 +190,11 @@ _dbus_pending_call_complete (DBusPendingCall *pending) } void -_dbus_pending_call_queue_timeout_error (DBusPendingCall *pending, - DBusConnection *connection) +_dbus_pending_call_queue_timeout_error_unlocked (DBusPendingCall *pending, + DBusConnection *connection) { + _dbus_assert (connection == pending->connection); + if (pending->timeout_link) { _dbus_connection_queue_synthesized_message_link (connection, @@ -205,7 +210,7 @@ _dbus_pending_call_queue_timeout_error (DBusPendingCall *pending, * @returns #TRUE if there is a timeout or #FALSE if not */ dbus_bool_t -_dbus_pending_call_is_timeout_added (DBusPendingCall *pending) +_dbus_pending_call_is_timeout_added_unlocked (DBusPendingCall *pending) { _dbus_assert (pending != NULL); @@ -220,8 +225,8 @@ _dbus_pending_call_is_timeout_added (DBusPendingCall *pending) * @param is_added whether or not a timeout is added */ void -_dbus_pending_call_set_timeout_added (DBusPendingCall *pending, - dbus_bool_t is_added) +_dbus_pending_call_set_timeout_added_unlocked (DBusPendingCall *pending, + dbus_bool_t is_added) { _dbus_assert (pending != NULL); @@ -236,7 +241,7 @@ _dbus_pending_call_set_timeout_added (DBusPendingCall *pending, * @returns a timeout object */ DBusTimeout * -_dbus_pending_call_get_timeout (DBusPendingCall *pending) +_dbus_pending_call_get_timeout_unlocked (DBusPendingCall *pending) { _dbus_assert (pending != NULL); @@ -250,7 +255,7 @@ _dbus_pending_call_get_timeout (DBusPendingCall *pending) * @returns a serial number for the reply or 0 */ dbus_uint32_t -_dbus_pending_call_get_reply_serial (DBusPendingCall *pending) +_dbus_pending_call_get_reply_serial_unlocked (DBusPendingCall *pending) { _dbus_assert (pending != NULL); @@ -264,8 +269,8 @@ _dbus_pending_call_get_reply_serial (DBusPendingCall *pending) * @param serial the serial number */ void -_dbus_pending_call_set_reply_serial (DBusPendingCall *pending, - dbus_uint32_t serial) +_dbus_pending_call_set_reply_serial_unlocked (DBusPendingCall *pending, + dbus_uint32_t serial) { _dbus_assert (pending != NULL); _dbus_assert (pending->reply_serial == 0); @@ -274,17 +279,32 @@ _dbus_pending_call_set_reply_serial (DBusPendingCall *pending, } /** - * Gets the connection associated with this pending call + * Gets the connection associated with this pending call. + * + * @param pending the pending_call + * @returns the connection associated with the pending call + */ +DBusConnection * +_dbus_pending_call_get_connection_and_lock (DBusPendingCall *pending) +{ + _dbus_assert (pending != NULL); + + CONNECTION_LOCK (pending->connection); + return pending->connection; +} + +/** + * Gets the connection associated with this pending call. * * @param pending the pending_call * @returns the connection associated with the pending call */ DBusConnection * -_dbus_pending_call_get_connection (DBusPendingCall *pending) +_dbus_pending_call_get_connection_unlocked (DBusPendingCall *pending) { _dbus_assert (pending != NULL); - return pending->connection; + return pending->connection; } /** @@ -296,9 +316,9 @@ _dbus_pending_call_get_connection (DBusPendingCall *pending) * @return #FALSE on OOM */ dbus_bool_t -_dbus_pending_call_set_timeout_error (DBusPendingCall *pending, - DBusMessage *message, - dbus_uint32_t serial) +_dbus_pending_call_set_timeout_error_unlocked (DBusPendingCall *pending, + DBusMessage *message, + dbus_uint32_t serial) { DBusList *reply_link; DBusMessage *reply; @@ -321,7 +341,7 @@ _dbus_pending_call_set_timeout_error (DBusPendingCall *pending, pending->timeout_link = reply_link; - _dbus_pending_call_set_reply_serial (pending, serial); + _dbus_pending_call_set_reply_serial_unlocked (pending, serial); return TRUE; } @@ -346,6 +366,21 @@ _dbus_pending_call_set_timeout_error (DBusPendingCall *pending, * Opaque data type representing a message pending. */ +/** + * Increments the reference count on a pending call, + * while the lock on its connection is already held. + * + * @param pending the pending call object + * @returns the pending call object + */ +DBusPendingCall * +_dbus_pending_call_ref_unlocked (DBusPendingCall *pending) +{ + pending->refcount.value += 1; + + return pending; +} + /** * Increments the reference count on a pending call. * @@ -357,11 +392,87 @@ dbus_pending_call_ref (DBusPendingCall *pending) { _dbus_return_val_if_fail (pending != NULL, NULL); + /* The connection lock is better than the global + * lock in the atomic increment fallback + */ +#ifdef DBUS_HAVE_ATOMIC_INT _dbus_atomic_inc (&pending->refcount); +#else + CONNECTION_LOCK (pending->connection); + _dbus_assert (pending->refcount.value > 0); + pending->refcount.value += 1; + CONNECTION_UNLOCK (pending->connection); +#endif + return pending; } +static void +_dbus_pending_call_last_unref (DBusPendingCall *pending) +{ + DBusConnection *connection; + + /* If we get here, we should be already detached + * from the connection, or never attached. + */ + _dbus_assert (!pending->timeout_added); + + connection = pending->connection; + + /* this assumes we aren't holding connection lock... */ + _dbus_data_slot_list_free (&pending->slot_list); + + if (pending->timeout != NULL) + _dbus_timeout_unref (pending->timeout); + + if (pending->timeout_link) + { + dbus_message_unref ((DBusMessage *)pending->timeout_link->data); + _dbus_list_free_link (pending->timeout_link); + pending->timeout_link = NULL; + } + + if (pending->reply) + { + dbus_message_unref (pending->reply); + pending->reply = NULL; + } + + dbus_free (pending); + + dbus_pending_call_free_data_slot (¬ify_user_data_slot); + + /* connection lock should not be held. */ + /* Free the connection last to avoid a weird state while + * calling out to application code where the pending exists + * but not the connection. + */ + dbus_connection_unref (connection); +} + +/** + * Decrements the reference count on a pending call, + * freeing it if the count reaches 0. Assumes + * connection lock is already held. + * + * @param pending the pending call object + */ +void +_dbus_pending_call_unref_and_unlock (DBusPendingCall *pending) +{ + dbus_bool_t last_unref; + + _dbus_assert (pending->refcount.value > 0); + + pending->refcount.value -= 1; + last_unref = pending->refcount.value == 0; + + CONNECTION_UNLOCK (pending->connection); + if (last_unref) + _dbus_pending_call_last_unref (pending); +} + /** * Decrements the reference count on a pending call, * freeing it if the count reaches 0. @@ -375,40 +486,21 @@ dbus_pending_call_unref (DBusPendingCall *pending) _dbus_return_if_fail (pending != NULL); + /* More efficient to use the connection lock instead of atomic + * int fallback if we lack atomic int decrement + */ +#ifdef DBUS_HAVE_ATOMIC_INT last_unref = (_dbus_atomic_dec (&pending->refcount) == 1); - +#else + CONNECTION_LOCK (pending->connection); + _dbus_assert (pending->refcount.value > 0); + pending->refcount.value -= 1; + last_unref = pending->refcount.value == 0; + CONNECTION_UNLOCK (pending->connection); +#endif + if (last_unref) - { - /* If we get here, we should be already detached - * from the connection, or never attached. - */ - _dbus_assert (!pending->timeout_added); - - dbus_connection_unref (pending->connection); - - /* this assumes we aren't holding connection lock... */ - _dbus_data_slot_list_free (&pending->slot_list); - - if (pending->timeout != NULL) - _dbus_timeout_unref (pending->timeout); - - if (pending->timeout_link) - { - dbus_message_unref ((DBusMessage *)pending->timeout_link->data); - _dbus_list_free_link (pending->timeout_link); - pending->timeout_link = NULL; - } - - if (pending->reply) - { - dbus_message_unref (pending->reply); - pending->reply = NULL; - } - - dbus_free (pending); - - dbus_pending_call_free_data_slot (¬ify_user_data_slot); - } + _dbus_pending_call_last_unref(pending); } /** @@ -429,13 +521,17 @@ dbus_pending_call_set_notify (DBusPendingCall *pending, { _dbus_return_val_if_fail (pending != NULL, FALSE); + CONNECTION_LOCK (pending->connection); + /* could invoke application code! */ - if (!dbus_pending_call_set_data (pending, notify_user_data_slot, - user_data, free_user_data)) + if (!_dbus_pending_call_set_data_unlocked (pending, notify_user_data_slot, + user_data, free_user_data)) return FALSE; pending->function = function; + CONNECTION_UNLOCK (pending->connection); + return TRUE; } @@ -451,23 +547,40 @@ dbus_pending_call_set_notify (DBusPendingCall *pending, void dbus_pending_call_cancel (DBusPendingCall *pending) { - if (pending->connection) - _dbus_connection_remove_pending_call (pending->connection, - pending); + _dbus_connection_remove_pending_call (pending->connection, + pending); } /** * Checks whether the pending call has received a reply - * yet, or not. + * yet, or not. Assumes connection lock is held. * - * @todo not thread safe? I guess it has to lock though it sucks + * @param pending the pending call + * @returns #TRUE if a reply has been received + */ +dbus_bool_t +_dbus_pending_call_get_completed_unlocked (DBusPendingCall *pending) +{ + return pending->completed; +} + +/** + * Checks whether the pending call has received a reply + * yet, or not. * * @param pending the pending call - * @returns #TRUE if a reply has been received */ + * @returns #TRUE if a reply has been received + */ dbus_bool_t dbus_pending_call_get_completed (DBusPendingCall *pending) { - return pending->completed; + dbus_bool_t completed; + + CONNECTION_LOCK (pending->connection); + completed = pending->completed; + CONNECTION_UNLOCK (pending->connection); + + return completed; } /** @@ -486,10 +599,14 @@ dbus_pending_call_steal_reply (DBusPendingCall *pending) _dbus_return_val_if_fail (pending->completed, NULL); _dbus_return_val_if_fail (pending->reply != NULL, NULL); + + CONNECTION_LOCK (pending->connection); message = pending->reply; pending->reply = NULL; + CONNECTION_UNLOCK (pending->connection); + return message; } @@ -553,7 +670,7 @@ void dbus_pending_call_free_data_slot (dbus_int32_t *slot_p) { _dbus_return_if_fail (*slot_p >= 0); - + _dbus_data_slot_allocator_free (&slot_allocator, slot_p); } @@ -571,29 +688,62 @@ dbus_pending_call_free_data_slot (dbus_int32_t *slot_p) * @returns #TRUE if there was enough memory to store the data */ dbus_bool_t -dbus_pending_call_set_data (DBusPendingCall *pending, - dbus_int32_t slot, - void *data, - DBusFreeFunction free_data_func) +_dbus_pending_call_set_data_unlocked (DBusPendingCall *pending, + dbus_int32_t slot, + void *data, + DBusFreeFunction free_data_func) { DBusFreeFunction old_free_func; void *old_data; dbus_bool_t retval; - _dbus_return_val_if_fail (pending != NULL, FALSE); - _dbus_return_val_if_fail (slot >= 0, FALSE); - retval = _dbus_data_slot_list_set (&slot_allocator, &pending->slot_list, slot, data, free_data_func, &old_free_func, &old_data); + /* Drop locks to call out to app code */ + CONNECTION_UNLOCK (pending->connection); + if (retval) { if (old_free_func) (* old_free_func) (old_data); } + CONNECTION_LOCK (pending->connection); + + return retval; +} + +/** + * Stores a pointer on a #DBusPendingCall, along + * with an optional function to be used for freeing + * the data when the data is set again, or when + * the pending call is finalized. The slot number + * must have been allocated with dbus_pending_call_allocate_data_slot(). + * + * @param pending the pending_call + * @param slot the slot number + * @param data the data to store + * @param free_data_func finalizer function for the data + * @returns #TRUE if there was enough memory to store the data + */ +dbus_bool_t +dbus_pending_call_set_data (DBusPendingCall *pending, + dbus_int32_t slot, + void *data, + DBusFreeFunction free_data_func) +{ + dbus_bool_t retval; + + _dbus_return_val_if_fail (pending != NULL, FALSE); + _dbus_return_val_if_fail (slot >= 0, FALSE); + + + CONNECTION_LOCK (pending->connection); + retval = _dbus_pending_call_set_data_unlocked (pending, slot, data, free_data_func); + CONNECTION_UNLOCK (pending->connection); return retval; } @@ -613,9 +763,11 @@ dbus_pending_call_get_data (DBusPendingCall *pending, _dbus_return_val_if_fail (pending != NULL, NULL); + CONNECTION_LOCK (pending->connection); res = _dbus_data_slot_list_get (&slot_allocator, &pending->slot_list, slot); + CONNECTION_UNLOCK (pending->connection); return res; } -- cgit