From 7020b573764bb86551d329e867c2e87172424c9b Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Sun, 1 Oct 2006 17:21:03 +0000 Subject: 2006-10-01 Havoc Pennington * test/test-service.c (path_message_func): remove broken extra unref that was hidden by the bugs in dbus-connection.c/dbus-bus.c * test/test-shell-service.c (path_message_func): same fix * dbus/dbus-connection.c (_dbus_connection_get_dispatch_status_unlocked): break up the function a little for clarity and fix the notification of dbus-bus.c to not require dispatch to be complete * dbus/dbus-connection.c (dbus_connection_unref): improve the warning when you try to finalize an open connection. --- dbus/dbus-bus.c | 9 ++- dbus/dbus-connection.c | 145 +++++++++++++++++++++++++++++++------------------ dbus/dbus-sysdeps.c | 2 +- 3 files changed, 101 insertions(+), 55 deletions(-) (limited to 'dbus') diff --git a/dbus/dbus-bus.c b/dbus/dbus-bus.c index f6918fd5..9eb2c526 100644 --- a/dbus/dbus-bus.c +++ b/dbus/dbus-bus.c @@ -257,6 +257,10 @@ bus_data_free (void *data) int i; _DBUS_LOCK (bus); /* We may be stored in more than one slot */ + /* This should now be impossible - these slots are supposed to + * be cleared on disconnect, so should not need to be cleared on + * finalize + */ i = 0; while (i < N_BUS_TYPES) { @@ -427,7 +431,10 @@ internal_bus_get (DBusBusType type, if (!private) { - /* get a weak ref to the connection */ + /* store a weak ref to the connection (dbus-connection.c is + * supposed to have a strong ref that it drops on disconnect, + * since this is a shared connection) + */ bus_connections[type] = connection; } diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 41058751..a94e0b2e 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -1964,7 +1964,8 @@ _dbus_connection_last_unref (DBusConnection *connection) * For shared connections, libdbus will own a reference * as long as the connection is connected, so you can know that either * you don't have the last reference, or it's OK to drop the last reference. - * Most connections are shared. + * Most connections are shared. dbus_connection_open() and dbus_bus_get() + * return shared connections. * * For private connections, the creator of the connection must arrange for * dbus_connection_close() to be called prior to dropping the last reference. @@ -2002,7 +2003,20 @@ dbus_connection_unref (DBusConnection *connection) #endif if (last_unref) - _dbus_connection_last_unref (connection); + { +#ifndef DBUS_DISABLE_CHECKS + if (_dbus_transport_get_is_connected (connection->transport)) + { + _dbus_warn ("The last reference on a connection was dropped without closing the connection. This is a bug. See dbus_connection_unref() documentation for details.\n"); + if (connection->shareable) + _dbus_warn ("Most likely, the application called unref() too many times and removed a reference belonging to libdbus, since this is a shared connection.\n"); + else + _dbus_warn ("Most likely, the application was supposed to call dbus_connection_close(), since this is a private connection.\n"); + return; + } +#endif + _dbus_connection_last_unref (connection); + } } /* @@ -2109,6 +2123,7 @@ dbus_connection_close (DBusConnection *connection) CONNECTION_LOCK (connection); +#ifndef DBUS_DISABLE_CHECKS if (connection->shareable) { CONNECTION_UNLOCK (connection); @@ -2116,7 +2131,8 @@ dbus_connection_close (DBusConnection *connection) _dbus_warn ("Applications must not close shared connections - see dbus_connection_close() docs. This is a bug in the application.\n"); return; } - +#endif + _dbus_connection_close_possibly_shared_and_unlock (connection); } @@ -3670,6 +3686,67 @@ _dbus_connection_failed_pop (DBusConnection *connection, connection->n_incoming += 1; } +/* Note this may be called multiple times since we don't track whether we already did it */ +static void +notify_disconnected_unlocked (DBusConnection *connection) +{ + HAVE_LOCK_CHECK (connection); + + /* Set the weakref in dbus-bus.c to NULL, so nobody will get a disconnected + * connection from dbus_bus_get(). We make the same guarantee for + * dbus_connection_open() but in a different way since we don't want to + * unref right here; we instead check for connectedness before returning + * the connection from the hash. + */ + _dbus_bus_notify_shared_connection_disconnected_unlocked (connection); + + /* Dump the outgoing queue, we aren't going to be able to + * send it now, and we'd like accessors like + * dbus_connection_get_outgoing_size() to be accurate. + */ + if (connection->n_outgoing > 0) + { + DBusList *link; + + _dbus_verbose ("Dropping %d outgoing messages since we're disconnected\n", + connection->n_outgoing); + + while ((link = _dbus_list_get_last_link (&connection->outgoing_messages))) + { + _dbus_connection_message_sent (connection, link->data); + } + } +} + +/* Note this may be called multiple times since we don't track whether we already did it */ +static DBusDispatchStatus +notify_disconnected_and_dispatch_complete_unlocked (DBusConnection *connection) +{ + HAVE_LOCK_CHECK (connection); + + if (connection->disconnect_message_link != NULL) + { + _dbus_verbose ("Sending disconnect message from %s\n", + _DBUS_FUNCTION_NAME); + + /* If we have pending calls, queue their timeouts - we want the Disconnected + * to be the last message, after these timeouts. + */ + connection_timeout_and_complete_all_pending_calls_unlocked (connection); + + /* We haven't sent the disconnect message already, + * and all real messages have been queued up. + */ + _dbus_connection_queue_synthesized_message_link (connection, + connection->disconnect_message_link); + connection->disconnect_message_link = NULL; + + return DBUS_DISPATCH_DATA_REMAINS; + } + + return DBUS_DISPATCH_COMPLETE; +} + static DBusDispatchStatus _dbus_connection_get_dispatch_status_unlocked (DBusConnection *connection) { @@ -3692,59 +3769,21 @@ _dbus_connection_get_dispatch_status_unlocked (DBusConnection *connection) if (!is_connected) { - /* It's possible this would be better done by having an - * explicit notification from - * _dbus_transport_disconnect() that would synchronously - * do this, instead of waiting for the next dispatch - * status check. However, probably not good to change - * until it causes a problem. + /* It's possible this would be better done by having an explicit + * notification from _dbus_transport_disconnect() that would + * synchronously do this, instead of waiting for the next dispatch + * status check. However, probably not good to change until it causes + * a problem. */ - - if (status == DBUS_DISPATCH_COMPLETE && - connection->disconnect_message_link) - { - _dbus_verbose ("Sending disconnect message from %s\n", - _DBUS_FUNCTION_NAME); - - /* If we have pending calls, queue their timeouts - we want the Disconnected - * to be the last message, after these timeouts. - */ - connection_timeout_and_complete_all_pending_calls_unlocked (connection); - - /* We haven't sent the disconnect message already, - * and all real messages have been queued up. - */ - _dbus_connection_queue_synthesized_message_link (connection, - connection->disconnect_message_link); - connection->disconnect_message_link = NULL; - - /* Set the weakref in dbus-bus.c to NULL, so nobody will get a disconnected - * connection from dbus_bus_get(). We make the same guarantee for - * dbus_connection_open() but in a different way since we don't want to - * unref right here; we instead check for connectedness before returning - * the connection from the hash. - */ - _dbus_bus_notify_shared_connection_disconnected_unlocked (connection); - - status = DBUS_DISPATCH_DATA_REMAINS; - } + notify_disconnected_unlocked (connection); - /* Dump the outgoing queue, we aren't going to be able to - * send it now, and we'd like accessors like - * dbus_connection_get_outgoing_size() to be accurate. + /* I'm not sure this is needed; the idea is that we want to + * queue the Disconnected only after we've read all the + * messages, but if we're disconnected maybe we are guaranteed + * to have read them all ? */ - if (connection->n_outgoing > 0) - { - DBusList *link; - - _dbus_verbose ("Dropping %d outgoing messages since we're disconnected\n", - connection->n_outgoing); - - while ((link = _dbus_list_get_last_link (&connection->outgoing_messages))) - { - _dbus_connection_message_sent (connection, link->data); - } - } + if (status == DBUS_DISPATCH_COMPLETE) + status = notify_disconnected_and_dispatch_complete_unlocked (connection); } if (status != DBUS_DISPATCH_COMPLETE) diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c index e8c03ef3..b7040abf 100644 --- a/dbus/dbus-sysdeps.c +++ b/dbus/dbus-sysdeps.c @@ -69,7 +69,7 @@ _dbus_abort (void) { /* don't use _dbus_warn here since it can _dbus_abort() */ fprintf (stderr, " Process %lu sleeping for gdb attach\n", (unsigned long) _dbus_getpid()); - _dbus_sleep_milliseconds (1000 * 60); + _dbus_sleep_milliseconds (1000 * 180); } abort (); -- cgit