diff options
| -rw-r--r-- | ChangeLog | 15 | ||||
| -rw-r--r-- | dbus/dbus-bus.c | 9 | ||||
| -rw-r--r-- | dbus/dbus-connection.c | 145 | ||||
| -rw-r--r-- | dbus/dbus-sysdeps.c | 2 | ||||
| -rw-r--r-- | test/test-service.c | 11 | ||||
| -rw-r--r-- | test/test-shell-service.c | 1 |
6 files changed, 121 insertions, 62 deletions
@@ -1,5 +1,20 @@ 2006-10-01 Havoc Pennington <hp@redhat.com> + * 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. + +2006-10-01 Havoc Pennington <hp@redhat.com> + * dbus/dbus-bus.c (internal_bus_get): only weak ref the connection; this means _dbus_bus_notify_shared_connection_disconnected_unlocked can be 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 (); diff --git a/test/test-service.c b/test/test-service.c index 0dbece08..bd2a4638 100644 --- a/test/test-service.c +++ b/test/test-service.c @@ -3,7 +3,7 @@ static DBusLoop *loop; static dbus_bool_t already_quit = FALSE; -static dbus_bool_t hello_from_self_reply_recived = FALSE; +static dbus_bool_t hello_from_self_reply_received = FALSE; static void quit (void) @@ -54,7 +54,7 @@ check_hello_from_self_reply (DBusPendingCall *pcall, if (type == DBUS_MESSAGE_TYPE_METHOD_RETURN) { const char *s; - printf ("Reply from HelloFromSelf recived\n"); + printf ("Reply from HelloFromSelf received\n"); if (!dbus_message_get_args (echo_message, &error, @@ -108,9 +108,9 @@ check_hello_from_self_reply (DBusPendingCall *pcall, dbus_error_free (&error); } else - _dbus_assert_not_reached ("Unexpected message recived\n"); + _dbus_assert_not_reached ("Unexpected message received\n"); - hello_from_self_reply_recived = TRUE; + hello_from_self_reply_received = TRUE; dbus_message_unref (reply); dbus_message_unref (echo_message); @@ -243,7 +243,6 @@ path_message_func (DBusConnection *connection, "org.freedesktop.TestSuite", "Exit")) { - dbus_connection_unref (connection); quit (); return DBUS_HANDLER_RESULT_HANDLED; } @@ -286,7 +285,7 @@ path_message_func (DBusConnection *connection, "HelloFromSelf")) { DBusMessage *reply; - printf ("Recived the HelloFromSelf message\n"); + printf ("Received the HelloFromSelf message\n"); reply = dbus_message_new_method_return (message); if (reply == NULL) diff --git a/test/test-shell-service.c b/test/test-shell-service.c index 08ed2077..21801c7b 100644 --- a/test/test-shell-service.c +++ b/test/test-shell-service.c @@ -85,7 +85,6 @@ path_message_func (DBusConnection *connection, "org.freedesktop.TestSuite", "Exit")) { - dbus_connection_unref (connection); quit (); return DBUS_HANDLER_RESULT_HANDLED; } |