From: John (J5) Palmieri Date: Wed, 6 Sep 2006 22:00:07 +0000 (+0000) Subject: * doc/TODO: X-Git-Tag: dbus-0.93~18 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1eae184450a585f10c8988613e0f7259e1d6066a;p=platform%2Fupstream%2Fdbus.git * doc/TODO: - Remove pending call locking todo item - dbus_connection_open now holds hard ref. Remove todo item - do proper locking on _dbus_bus_check_connection_and_unref and handle DBUS_BUS_STARTER. Remove todo item - Warn on closing of a shared connection. Remove todo item * bus/bus.c, bus/connection.c, bus/dispatch.c, dbus/dbus-bus.c, dbus/dbus-connection.c: Use the dbus_connection_close_internal so we don't get the warning when closing shared connections * test/test-service.c, test/test-shell-service.c: Applications don't close shared connections themselves so we unref instead of close * test/test-utils.c (test_connection_shutdown): Close the connection * dbus/dbus-bus.c (_dbus_bus_check_connection_and_unref): Changed to _dbus_bus_check_connection_and_unref_unlocked since we only call this method on a locked connection. Make sure we call _dbus_connection_unref_unlocked instead of dbus_connection_unref also. Handle DBUS_BUS_STARTER correctly * dbus/dbus-connection.c (connection_record_shared_unlocked): Mark as shared and hard ref the connection (connection_forget_shared_unlocked): Remove the hard ref from the connection (_dbus_connection_close_internal_and_unlock): New internal function which takes a locked connection and unlocks it after closing it (_dbus_connection_close_internal): New internal function which acts like the origonal dbus_connection_close method by grabbing a connection lock and calling _dbus_connection_close_internal_and_unlock (dbus_connection_close): Public close method, warns when the app trys to close a shared connection --- diff --git a/ChangeLog b/ChangeLog index 247f3c9..6dfff6e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,43 @@ 2006-09-06 John (J5) Palmieri + * doc/TODO: + - Remove pending call locking todo item + - dbus_connection_open now holds hard ref. Remove todo item + - do proper locking on _dbus_bus_check_connection_and_unref + and handle DBUS_BUS_STARTER. Remove todo item + - Warn on closing of a shared connection. Remove todo item + + * bus/bus.c, bus/connection.c, bus/dispatch.c, dbus/dbus-bus.c, + dbus/dbus-connection.c: Use the dbus_connection_close_internal + so we don't get the warning when closing shared connections + + * test/test-service.c, test/test-shell-service.c: Applications + don't close shared connections themselves so we unref instead of + close + + * test/test-utils.c (test_connection_shutdown): Close the connection + + * dbus/dbus-bus.c (_dbus_bus_check_connection_and_unref): Changed to + _dbus_bus_check_connection_and_unref_unlocked since we only call this + method on a locked connection. + Make sure we call _dbus_connection_unref_unlocked instead of + dbus_connection_unref also. + Handle DBUS_BUS_STARTER correctly + + * dbus/dbus-connection.c (connection_record_shared_unlocked): + Mark as shared and hard ref the connection + (connection_forget_shared_unlocked): Remove the hard ref from the + connection + (_dbus_connection_close_internal_and_unlock): New internal function + which takes a locked connection and unlocks it after closing it + (_dbus_connection_close_internal): New internal function which acts + like the origonal dbus_connection_close method by grabbing a connection + lock and calling _dbus_connection_close_internal_and_unlock + (dbus_connection_close): Public close method, warns when the app + trys to close a shared connection + +2006-09-06 John (J5) Palmieri + * bus/driver.c: (bus_driver_generate_introspect_string): New method for populating a DBusString with the introspect data diff --git a/dbus/dbus-bus.c b/dbus/dbus-bus.c index eae4605..fd58fab 100644 --- a/dbus/dbus-bus.c +++ b/dbus/dbus-bus.c @@ -28,6 +28,7 @@ #include "dbus-message.h" #include "dbus-marshal-validate.h" #include "dbus-threads-internal.h" +#include "dbus-connection-internal.h" #include /** @@ -303,20 +304,29 @@ ensure_bus_data (DBusConnection *connection) } /* internal function that checks to see if this - is a shared bus connection and if it is unref it */ + is a shared connection owned by the bus and if it is unref it */ void -_dbus_bus_check_connection_and_unref (DBusConnection *connection) +_dbus_bus_check_connection_and_unref_unlocked (DBusConnection *connection) { + _DBUS_LOCK (bus); + if (bus_connections[DBUS_BUS_SYSTEM] == connection) { bus_connections[DBUS_BUS_SYSTEM] = NULL; - dbus_connection_unref (connection); + _dbus_connection_unref_unlocked (connection); } else if (bus_connections[DBUS_BUS_SESSION] == connection) { bus_connections[DBUS_BUS_SESSION] = NULL; - dbus_connection_unref (connection); + _dbus_connection_unref_unlocked (connection); } + else if (bus_connections[DBUS_BUS_STARTER] == connection) + { + bus_connections[DBUS_BUS_STARTER] = NULL; + _dbus_connection_unref_unlocked (connection); + } + + _DBUS_UNLOCK (bus); } static DBusConnection * @@ -394,7 +404,7 @@ internal_bus_get (DBusBusType type, if (!dbus_bus_register (connection, error)) { _DBUS_ASSERT_ERROR_IS_SET (error); - dbus_connection_close (connection); + _dbus_connection_close_internal (connection); dbus_connection_unref (connection); _DBUS_UNLOCK (bus); diff --git a/dbus/dbus-bus.h b/dbus/dbus-bus.h index c47af15..6cefe35 100644 --- a/dbus/dbus-bus.h +++ b/dbus/dbus-bus.h @@ -68,7 +68,7 @@ void dbus_bus_remove_match (DBusConnection *connection, const char *rule, DBusError *error); -void _dbus_bus_check_connection_and_unref (DBusConnection *connection); +void _dbus_bus_check_connection_and_unref_unlocked (DBusConnection *connection); DBUS_END_DECLS diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h index adf1786..4506f63 100644 --- a/dbus/dbus-connection-internal.h +++ b/dbus/dbus-connection-internal.h @@ -77,6 +77,7 @@ DBusConnection* _dbus_connection_new_for_transport (DBusTransport void _dbus_connection_do_iteration_unlocked (DBusConnection *connection, unsigned int flags, int timeout_milliseconds); +void _dbus_connection_close_internal (DBusConnection *connection); DBusPendingCall* _dbus_pending_call_new (DBusConnection *connection, int timeout_milliseconds, diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 06c08d0..8033c4a 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -239,7 +239,9 @@ struct DBusConnection char *server_guid; /**< GUID of server if we are in shared_connections, #NULL if server GUID is unknown or connection is private */ unsigned int shareable : 1; /**< #TRUE if connection can go in shared_connections once we know the GUID */ - + + unsigned int shared : 1; /** < #TRUE if connection is shared and we hold a ref to it */ + unsigned int dispatch_acquired : 1; /**< Someone has dispatch path (can drain incoming queue) */ unsigned int io_path_acquired : 1; /**< Someone has transport io path (can use the transport to read/write messages) */ @@ -1360,6 +1362,7 @@ shared_connections_shutdown (void *data) _DBUS_LOCK (shared_connections); _dbus_assert (_dbus_hash_table_get_n_entries (shared_connections) == 0); + _dbus_hash_table_unref (shared_connections); shared_connections = NULL; @@ -1474,6 +1477,10 @@ connection_record_shared_unlocked (DBusConnection *connection, } connection->server_guid = guid_in_connection; + connection->shared = TRUE; + + /* get a hard ref on this connection */ + dbus_connection_ref (connection); _dbus_verbose ("stored connection to %s to be shared\n", connection->server_guid); @@ -1489,7 +1496,7 @@ static void connection_forget_shared_unlocked (DBusConnection *connection) { HAVE_LOCK_CHECK (connection); - + if (connection->server_guid == NULL) return; @@ -1505,6 +1512,8 @@ connection_forget_shared_unlocked (DBusConnection *connection) dbus_free (connection->server_guid); connection->server_guid = NULL; + /* remove the hash ref */ + _dbus_connection_unref_unlocked (connection); _DBUS_UNLOCK (shared_connections); } @@ -1605,7 +1614,7 @@ _dbus_connection_open_internal (const char *address, !connection_record_shared_unlocked (connection, guid)) { _DBUS_SET_OOM (&tmp_error); - dbus_connection_close (connection); + _dbus_connection_close_internal (connection); dbus_connection_unref (connection); connection = NULL; } @@ -1896,6 +1905,32 @@ dbus_connection_unref (DBusConnection *connection) _dbus_connection_last_unref (connection); } +static void +_dbus_connection_close_internal_and_unlock (DBusConnection *connection) +{ + DBusDispatchStatus status; + + _dbus_verbose ("Disconnecting %p\n", connection); + + _dbus_transport_disconnect (connection->transport); + + _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); +} + +void +_dbus_connection_close_internal (DBusConnection *connection) +{ + _dbus_assert (connection != NULL); + _dbus_assert (connection->generation == _dbus_current_generation); + + CONNECTION_LOCK (connection); + _dbus_connection_close_internal_and_unlock (connection); +} + /** * Closes the connection, so no further data can be sent or received. * Any further attempts to send data will result in errors. This @@ -1907,27 +1942,29 @@ dbus_connection_unref (DBusConnection *connection) * dbus_connection_set_dispatch_status_function(), as the disconnect * message it generates needs to be dispatched. * + * If the connection is a shared connection we print out a warning that + * you can not close shared connection and we return. Internal calls + * should use _dbus_connection_close_internal() to close shared connections. + * * @param connection the connection. */ void dbus_connection_close (DBusConnection *connection) { - DBusDispatchStatus status; - _dbus_return_if_fail (connection != NULL); _dbus_return_if_fail (connection->generation == _dbus_current_generation); - _dbus_verbose ("Disconnecting %p\n", connection); - CONNECTION_LOCK (connection); - - _dbus_transport_disconnect (connection->transport); - _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); - status = _dbus_connection_get_dispatch_status_unlocked (connection); + if (connection->shared) + { + CONNECTION_UNLOCK (connection); - /* this calls out to user code */ - _dbus_connection_update_dispatch_status_and_unlock (connection, status); + _dbus_warn ("Applications can not close shared connections. Please fix this in your app. Ignoring close request and continuing."); + return; + } + + _dbus_connection_close_internal_and_unlock (connection); } static dbus_bool_t @@ -3823,7 +3860,7 @@ dbus_connection_dispatch (DBusConnection *connection) DBUS_INTERFACE_LOCAL, "Disconnected")) { - _dbus_bus_check_connection_and_unref (connection); + _dbus_bus_check_connection_and_unref_unlocked (connection); if (connection->exit_on_disconnect) { diff --git a/doc/TODO b/doc/TODO index 2ef7fde..d3ece32 100644 --- a/doc/TODO +++ b/doc/TODO @@ -21,21 +21,6 @@ Important for 1.0 - just before 1.0, try a HAVE_INT64=0 build and be sure it runs - - fix locking on DBusPendingCall - - - dbus_connection_open() is like dbus_bus_get() in that it returns a shared - connection; it needs the corresponding fix to hold a strong reference to - these connections. - - - _dbus_bus_check_connection_and_unref does not do proper locking and - doesn't handle all the shared connections, e.g. DBUS_BUS_STARTER - - - for both the dbus-bus.c shared connections and dbus_connection_open() - shared connections, it is probably appropriate to warn() if someone - calls close() on them ; essentially shared connections are not closeable - because it's unclear who would do the closing and when, short of - dbus_shutdown() - Important for 1.0 GLib Bindings === diff --git a/test/test-service.c b/test/test-service.c index 6a633b7..0dbece0 100644 --- a/test/test-service.c +++ b/test/test-service.c @@ -115,6 +115,7 @@ check_hello_from_self_reply (DBusPendingCall *pcall, dbus_message_unref (reply); dbus_message_unref (echo_message); dbus_pending_call_unref (pcall); + dbus_connection_unref (connection); } static DBusHandlerResult @@ -242,7 +243,7 @@ path_message_func (DBusConnection *connection, "org.freedesktop.TestSuite", "Exit")) { - dbus_connection_close (connection); + dbus_connection_unref (connection); quit (); return DBUS_HANDLER_RESULT_HANDLED; } @@ -319,7 +320,6 @@ filter_func (DBusConnection *connection, DBUS_INTERFACE_LOCAL, "Disconnected")) { - dbus_connection_close (connection); quit (); return DBUS_HANDLER_RESULT_HANDLED; } diff --git a/test/test-shell-service.c b/test/test-shell-service.c index 71b4baa..08ed207 100644 --- a/test/test-shell-service.c +++ b/test/test-shell-service.c @@ -85,7 +85,7 @@ path_message_func (DBusConnection *connection, "org.freedesktop.TestSuite", "Exit")) { - dbus_connection_close (connection); + dbus_connection_unref (connection); quit (); return DBUS_HANDLER_RESULT_HANDLED; } @@ -109,7 +109,6 @@ filter_func (DBusConnection *connection, DBUS_INTERFACE_LOCAL, "Disconnected")) { - dbus_connection_close (connection); quit (); return DBUS_HANDLER_RESULT_HANDLED; } diff --git a/test/test-utils.c b/test/test-utils.c index 9665eda..3577bf9 100644 --- a/test/test-utils.c +++ b/test/test-utils.c @@ -171,6 +171,8 @@ void test_connection_shutdown (DBusLoop *loop, DBusConnection *connection) { + _dbus_connection_close_internal (connection); + if (!dbus_connection_set_watch_functions (connection, NULL, NULL,