From ee27481d7b7d6d9a4f41b7d641a2618dedf676dd Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Sat, 26 Feb 2005 06:37:46 +0000 Subject: 2005-02-26 Havoc Pennington * doc/TODO: remove the "guid" item * test/glib/test-profile.c (no_bus_thread_func): use open_private (with_bus_thread_func): use open_private * dbus/dbus-connection.c (dbus_connection_open_private): new function that works like the old dbus_connection_open() (dbus_connection_open): now returns an existing connection if possible * dbus/dbus-server-unix.c (handle_new_client_fd_and_unlock): pass through the GUID to the transport * dbus/dbus-server.c (_dbus_server_init_base): keep around the GUID in hex-encoded form. * dbus/dbus-server-debug-pipe.c (_dbus_transport_debug_pipe_new): pass GUID argument in to the transport * dbus/dbus-transport-unix.c (_dbus_transport_new_for_fd): add guid argument * dbus/dbus-transport.c (_dbus_transport_init_base): add guid argument * dbus/dbus-auth.c (_dbus_auth_server_new): add guid argument --- ChangeLog | 28 +++ dbus/dbus-auth-script.c | 4 +- dbus/dbus-auth.c | 29 ++- dbus/dbus-auth.h | 2 +- dbus/dbus-connection.c | 382 ++++++++++++++++++++++++++++++++++++---- dbus/dbus-connection.h | 2 + dbus/dbus-internals.h | 3 +- dbus/dbus-server-debug-pipe.c | 4 +- dbus/dbus-server-protected.h | 5 +- dbus/dbus-server-unix.c | 2 +- dbus/dbus-server.c | 36 ++-- dbus/dbus-threads.c | 3 +- dbus/dbus-transport-protected.h | 2 +- dbus/dbus-transport-unix.c | 6 +- dbus/dbus-transport-unix.h | 2 +- dbus/dbus-transport.c | 213 +++++++++------------- dbus/dbus-transport.h | 3 +- doc/TODO | 25 ++- test/glib/test-profile.c | 4 +- 19 files changed, 553 insertions(+), 202 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0f8daf25..e51ea0c5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,31 @@ +2005-02-26 Havoc Pennington + + * doc/TODO: remove the "guid" item + + * test/glib/test-profile.c (no_bus_thread_func): use open_private + (with_bus_thread_func): use open_private + + * dbus/dbus-connection.c (dbus_connection_open_private): new + function that works like the old dbus_connection_open() + (dbus_connection_open): now returns an existing connection if + possible + + * dbus/dbus-server-unix.c (handle_new_client_fd_and_unlock): pass + through the GUID to the transport + + * dbus/dbus-server.c (_dbus_server_init_base): keep around the + GUID in hex-encoded form. + + * dbus/dbus-server-debug-pipe.c (_dbus_transport_debug_pipe_new): + pass GUID argument in to the transport + + * dbus/dbus-transport-unix.c (_dbus_transport_new_for_fd): add + guid argument + + * dbus/dbus-transport.c (_dbus_transport_init_base): add guid argument + + * dbus/dbus-auth.c (_dbus_auth_server_new): add guid argument + 2005-02-25 Havoc Pennington * doc/dbus-specification.xml: document the GUID thing diff --git a/dbus/dbus-auth-script.c b/dbus/dbus-auth-script.c index f32f1c1d..10bf7038 100644 --- a/dbus/dbus-auth-script.c +++ b/dbus/dbus-auth-script.c @@ -231,10 +231,12 @@ _dbus_auth_script_run (const DBusString *filename) DBusString from_auth; DBusAuthState state; DBusString context; + DBusString guid; retval = FALSE; auth = NULL; + _dbus_string_init_const (&guid, "5fa01f4202cd837709a3274ca0df9d00"); _dbus_string_init_const (&context, "org_freedesktop_test"); if (!_dbus_string_init (&file)) @@ -334,7 +336,7 @@ _dbus_auth_script_run (const DBusString *filename) goto out; } - auth = _dbus_auth_server_new (); + auth = _dbus_auth_server_new (&guid); if (auth == NULL) { _dbus_warn ("no memory to create DBusAuth\n"); diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c index 6412185b..2a1bbd4c 100644 --- a/dbus/dbus-auth.c +++ b/dbus/dbus-auth.c @@ -211,6 +211,8 @@ typedef struct int failures; /**< Number of times client has been rejected */ int max_failures; /**< Number of times we reject before disconnect */ + + DBusString guid; /**< Our globally unique ID in hex encoding */ } DBusAuthServer; @@ -1923,20 +1925,35 @@ process_command (DBusAuth *auth) * @returns the new object or #NULL if no memory */ DBusAuth* -_dbus_auth_server_new (void) +_dbus_auth_server_new (const DBusString *guid) { DBusAuth *auth; DBusAuthServer *server_auth; + DBusString guid_copy; - auth = _dbus_auth_new (sizeof (DBusAuthServer)); - if (auth == NULL) + if (!_dbus_string_init (&guid_copy)) return NULL; + if (!_dbus_string_copy (guid, 0, &guid_copy, 0)) + { + _dbus_string_free (&guid_copy); + return NULL; + } + + auth = _dbus_auth_new (sizeof (DBusAuthServer)); + if (auth == NULL) + { + _dbus_string_free (&guid_copy); + return NULL; + } + auth->side = auth_side_server; auth->state = &server_state_waiting_for_auth; server_auth = DBUS_AUTH_SERVER (auth); + server_auth->guid = guid_copy; + /* perhaps this should be per-mechanism with a lower * max */ @@ -2012,6 +2029,12 @@ _dbus_auth_unref (DBusAuth *auth) { _dbus_list_clear (& DBUS_AUTH_CLIENT (auth)->mechs_to_try); } + else + { + _dbus_assert (DBUS_AUTH_IS_SERVER (auth)); + + _dbus_string_free (& DBUS_AUTH_SERVER (auth)->guid); + } if (auth->keyring) _dbus_keyring_unref (auth->keyring); diff --git a/dbus/dbus-auth.h b/dbus/dbus-auth.h index 715f6d17..6ed5040e 100644 --- a/dbus/dbus-auth.h +++ b/dbus/dbus-auth.h @@ -41,7 +41,7 @@ typedef enum DBUS_AUTH_STATE_AUTHENTICATED } DBusAuthState; -DBusAuth* _dbus_auth_server_new (void); +DBusAuth* _dbus_auth_server_new (const DBusString *guid); DBusAuth* _dbus_auth_client_new (void); DBusAuth* _dbus_auth_ref (DBusAuth *auth); void _dbus_auth_unref (DBusAuth *auth); diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 9c57edc9..92e0331b 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -233,6 +233,10 @@ struct DBusConnection * for the global linked list mempool lock */ DBusObjectTree *objects; /**< Object path handlers registered with this connection */ + + 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 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) */ @@ -1175,6 +1179,7 @@ _dbus_connection_new_for_transport (DBusTransport *transport) connection->last_dispatch_status = DBUS_DISPATCH_COMPLETE; /* so we're notified first time there's data */ connection->objects = objects; connection->exit_on_disconnect = FALSE; + connection->shareable = FALSE; #ifndef DBUS_DISABLE_CHECKS connection->generation = _dbus_current_generation; #endif @@ -1363,6 +1368,301 @@ _dbus_connection_handle_watch (DBusWatch *watch, return retval; } +_DBUS_DEFINE_GLOBAL_LOCK (shared_connections); +static DBusHashTable *shared_connections = NULL; + +static void +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; + + _DBUS_UNLOCK (shared_connections); +} + +static dbus_bool_t +connection_lookup_shared (DBusAddressEntry *entry, + DBusConnection **result) +{ + _dbus_verbose ("checking for existing connection\n"); + + *result = NULL; + + _DBUS_LOCK (shared_connections); + + if (shared_connections == NULL) + { + _dbus_verbose ("creating shared_connections hash table\n"); + + shared_connections = _dbus_hash_table_new (DBUS_HASH_STRING, + dbus_free, + NULL); + if (shared_connections == NULL) + { + _DBUS_UNLOCK (shared_connections); + return FALSE; + } + + if (!_dbus_register_shutdown_func (shared_connections_shutdown, NULL)) + { + _dbus_hash_table_unref (shared_connections); + shared_connections = NULL; + _DBUS_UNLOCK (shared_connections); + return FALSE; + } + + _dbus_verbose (" successfully created shared_connections\n"); + + _DBUS_UNLOCK (shared_connections); + return TRUE; /* no point looking up in the hash we just made */ + } + else + { + const char *guid; + + guid = dbus_address_entry_get_value (entry, "guid"); + + if (guid != NULL) + { + *result = _dbus_hash_table_lookup_string (shared_connections, + guid); + + if (*result) + { + /* The DBusConnection can't have been disconnected + * between the lookup and this code, because the + * disconnection will take the shared_connections lock to + * remove the connection. It can't have been finalized + * since you have to disconnect prior to finalize. + * + * Thus it's safe to ref the connection. + */ + dbus_connection_ref (*result); + + _dbus_verbose ("looked up existing connection to server guid %s\n", + guid); + } + } + + _DBUS_UNLOCK (shared_connections); + return TRUE; + } +} + +static dbus_bool_t +connection_record_shared_unlocked (DBusConnection *connection, + const char *guid) +{ + char *guid_key; + char *guid_in_connection; + + /* A separate copy of the key is required in the hash table, because + * we don't have a lock on the connection when we are doing a hash + * lookup. + */ + + _dbus_assert (connection->server_guid == NULL); + _dbus_assert (connection->shareable); + + guid_key = _dbus_strdup (guid); + if (guid_key == NULL) + return FALSE; + + guid_in_connection = _dbus_strdup (guid); + if (guid_in_connection == NULL) + { + dbus_free (guid_key); + return FALSE; + } + + _DBUS_LOCK (shared_connections); + _dbus_assert (shared_connections != NULL); + + if (!_dbus_hash_table_insert_string (shared_connections, + guid_key, connection)) + { + dbus_free (guid_key); + dbus_free (guid_in_connection); + _DBUS_UNLOCK (shared_connections); + return FALSE; + } + + connection->server_guid = guid_in_connection; + + _dbus_verbose ("stored connection to %s to be shared\n", + connection->server_guid); + + _DBUS_UNLOCK (shared_connections); + + _dbus_assert (connection->server_guid != NULL); + + return TRUE; +} + +static void +connection_forget_shared_unlocked (DBusConnection *connection) +{ + HAVE_LOCK_CHECK (connection); + + if (connection->server_guid == NULL) + return; + + _dbus_verbose ("dropping connection to %s out of the shared table\n", + connection->server_guid); + + _DBUS_LOCK (shared_connections); + + if (!_dbus_hash_table_remove_string (shared_connections, + connection->server_guid)) + _dbus_assert_not_reached ("connection was not in the shared table"); + + dbus_free (connection->server_guid); + connection->server_guid = NULL; + + _DBUS_UNLOCK (shared_connections); +} + +static DBusConnection* +connection_try_from_address_entry (DBusAddressEntry *entry, + DBusError *error) +{ + DBusTransport *transport; + DBusConnection *connection; + + transport = _dbus_transport_open (entry, error); + + if (transport == NULL) + { + _DBUS_ASSERT_ERROR_IS_SET (error); + return NULL; + } + + connection = _dbus_connection_new_for_transport (transport); + + _dbus_transport_unref (transport); + + if (connection == NULL) + { + _DBUS_SET_OOM (error); + return NULL; + } + +#ifndef DBUS_DISABLE_CHECKS + _dbus_assert (!connection->have_connection_lock); +#endif + return connection; +} + +/* + * If the shared parameter is true, then any existing connection will + * be used (and if a new connection is created, it will be available + * for use by others). If the shared parameter is false, a new + * connection will always be created, and the new connection will + * never be returned to other callers. + * + * @param address the address + * @param shared whether the connection is shared or private + * @param error error return + * @returns the connection or #NULL on error + */ +static DBusConnection* +_dbus_connection_open_internal (const char *address, + dbus_bool_t shared, + DBusError *error) +{ + DBusConnection *connection; + DBusAddressEntry **entries; + DBusError tmp_error; + DBusError first_error; + int len, i; + + _DBUS_ASSERT_ERROR_IS_CLEAR (error); + + _dbus_verbose ("opening %s connection to: %s\n", + shared ? "shared" : "private", address); + + if (!dbus_parse_address (address, &entries, &len, error)) + return NULL; + + _DBUS_ASSERT_ERROR_IS_CLEAR (error); + + connection = NULL; + + dbus_error_init (&tmp_error); + dbus_error_init (&first_error); + for (i = 0; i < len; i++) + { + if (shared) + { + if (!connection_lookup_shared (entries[i], &connection)) + _DBUS_SET_OOM (&tmp_error); + } + + if (connection == NULL) + { + connection = connection_try_from_address_entry (entries[i], + &tmp_error); + + if (connection != NULL && shared) + { + const char *guid; + + connection->shareable = TRUE; + + guid = dbus_address_entry_get_value (entries[i], "guid"); + + /* we don't have a connection lock but we know nobody + * else has a handle to the connection + */ + + if (guid && + !connection_record_shared_unlocked (connection, guid)) + { + _DBUS_SET_OOM (&tmp_error); + dbus_connection_disconnect (connection); + dbus_connection_unref (connection); + connection = NULL; + } + + /* but as of now the connection is possibly shared + * since another thread could have pulled it from the table + */ + } + } + + if (connection) + break; + + _DBUS_ASSERT_ERROR_IS_SET (&tmp_error); + + if (i == 0) + dbus_move_error (&tmp_error, &first_error); + else + dbus_error_free (&tmp_error); + } + + /* NOTE we don't have a lock on a possibly-shared connection object */ + + _DBUS_ASSERT_ERROR_IS_CLEAR (error); + _DBUS_ASSERT_ERROR_IS_CLEAR (&tmp_error); + + if (connection == NULL) + { + _DBUS_ASSERT_ERROR_IS_SET (&first_error); + dbus_move_error (&first_error, error); + } + else + { + dbus_error_free (&first_error); + } + + dbus_address_entries_free (entries); + return connection; +} + /** @} */ /** @@ -1372,16 +1672,19 @@ _dbus_connection_handle_watch (DBusWatch *watch, */ /** - * Opens a new connection to a remote address. + * Gets a connection to a remote address. If a connection to the given + * address already exists, returns the existing connection with its + * reference count incremented. Otherwise, returns a new connection + * and saves the new connection for possible re-use if a future call + * to dbus_connection_open() asks to connect to the same server. * - * @todo specify what the address parameter is. Right now - * it's just the name of a UNIX domain socket. It should be - * something more complex that encodes which transport to use. + * Use dbus_connection_open_private() to get a dedicated connection + * not shared with other callers of dbus_connection_open(). * - * If the open fails, the function returns #NULL, and provides - * a reason for the failure in the result parameter. Pass - * #NULL for the result parameter if you aren't interested - * in the reason for failure. + * If the open fails, the function returns #NULL, and provides a + * reason for the failure in the error parameter. Pass #NULL for the + * error parameter if you aren't interested in the reason for + * failure. * * @param address the address. * @param error address where an error can be returned. @@ -1392,31 +1695,44 @@ dbus_connection_open (const char *address, DBusError *error) { DBusConnection *connection; - DBusTransport *transport; _dbus_return_val_if_fail (address != NULL, NULL); _dbus_return_val_if_error_is_set (error, NULL); - - transport = _dbus_transport_open (address, error); - if (transport == NULL) - { - _DBUS_ASSERT_ERROR_IS_SET (error); - return NULL; - } - - connection = _dbus_connection_new_for_transport (transport); - _dbus_transport_unref (transport); - - if (connection == NULL) - { - dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); - return NULL; - } + connection = _dbus_connection_open_internal (address, + TRUE, + error); + + return connection; +} + +/** + * Opens a new, dedicated connection to a remote address. Unlike + * dbus_connection_open(), always creates a new connection. + * This connection will not be saved or recycled by libdbus. + * + * If the open fails, the function returns #NULL, and provides a + * reason for the failure in the error parameter. Pass #NULL for the + * error parameter if you aren't interested in the reason for + * failure. + * + * @param address the address. + * @param error address where an error can be returned. + * @returns new connection, or #NULL on failure. + */ +DBusConnection* +dbus_connection_open_private (const char *address, + DBusError *error) +{ + DBusConnection *connection; + + _dbus_return_val_if_fail (address != NULL, NULL); + _dbus_return_val_if_error_is_set (error, NULL); + + connection = _dbus_connection_open_internal (address, + FALSE, + error); -#ifndef DBUS_DISABLE_CHECKS - _dbus_assert (!connection->have_connection_lock); -#endif return connection; } @@ -1479,7 +1795,8 @@ _dbus_connection_last_unref (DBusConnection *connection) * you won't get the disconnected message. */ _dbus_assert (!_dbus_transport_get_is_connected (connection->transport)); - + _dbus_assert (connection->server_guid == NULL); + /* ---- We're going to call various application callbacks here, hope it doesn't break anything... */ _dbus_object_tree_free_all_unlocked (connection->objects); @@ -1529,7 +1846,7 @@ _dbus_connection_last_unref (DBusConnection *connection) _dbus_list_clear (&connection->incoming_messages); _dbus_counter_unref (connection->outgoing_counter); - + _dbus_transport_unref (connection->transport); if (connection->disconnect_message_link) @@ -1620,6 +1937,7 @@ dbus_connection_disconnect (DBusConnection *connection) _dbus_verbose ("Disconnecting %p\n", connection); CONNECTION_LOCK (connection); + _dbus_transport_disconnect (connection->transport); _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME); @@ -2838,7 +3156,9 @@ _dbus_connection_get_dispatch_status_unlocked (DBusConnection *connection) { _dbus_verbose ("Sending disconnect message from %s\n", _DBUS_FUNCTION_NAME); - + + connection_forget_shared_unlocked (connection); + /* We haven't sent the disconnect message already, * and all real messages have been queued up. */ diff --git a/dbus/dbus-connection.h b/dbus/dbus-connection.h index a74ba410..0dc1b617 100644 --- a/dbus/dbus-connection.h +++ b/dbus/dbus-connection.h @@ -89,6 +89,8 @@ typedef DBusHandlerResult (* DBusHandleMessageFunction) (DBusConnection *con DBusConnection* dbus_connection_open (const char *address, DBusError *error); +DBusConnection* dbus_connection_open_private (const char *address, + DBusError *error); DBusConnection* dbus_connection_ref (DBusConnection *connection); void dbus_connection_unref (DBusConnection *connection); void dbus_connection_disconnect (DBusConnection *connection); diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index b141c23d..88530947 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -280,7 +280,8 @@ _DBUS_DECLARE_GLOBAL_LOCK (bus); _DBUS_DECLARE_GLOBAL_LOCK (shutdown_funcs); _DBUS_DECLARE_GLOBAL_LOCK (system_users); _DBUS_DECLARE_GLOBAL_LOCK (message_cache); -#define _DBUS_N_GLOBAL_LOCKS (10) +_DBUS_DECLARE_GLOBAL_LOCK (shared_connections); +#define _DBUS_N_GLOBAL_LOCKS (11) dbus_bool_t _dbus_threads_init_debug (void); diff --git a/dbus/dbus-server-debug-pipe.c b/dbus/dbus-server-debug-pipe.c index 090274aa..9c147c6c 100644 --- a/dbus/dbus-server-debug-pipe.c +++ b/dbus/dbus-server-debug-pipe.c @@ -270,9 +270,9 @@ _dbus_transport_debug_pipe_new (const char *server_name, _dbus_string_free (&address); client_fd = -1; - + server_transport = _dbus_transport_new_for_fd (server_fd, - TRUE, NULL); + &server->guid_hex, NULL); if (server_transport == NULL) { _dbus_transport_unref (client_transport); diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h index dbbc3c62..7d09f64b 100644 --- a/dbus/dbus-server-protected.h +++ b/dbus/dbus-server-protected.h @@ -31,6 +31,7 @@ #include #include #include +#include DBUS_BEGIN_DECLS @@ -67,7 +68,9 @@ struct DBusServer const DBusServerVTable *vtable; /**< Virtual methods for this instance. */ DBusMutex *mutex; /**< Lock on the server object */ - DBusGUID guid; /**< Globally unique ID of server */ + DBusGUID guid; /**< Globally unique ID of server */ + + DBusString guid_hex; /**< Hex-encoded version of GUID */ DBusWatchList *watches; /**< Our watches */ DBusTimeoutList *timeouts; /**< Our timeouts */ diff --git a/dbus/dbus-server-unix.c b/dbus/dbus-server-unix.c index 0aef536e..a72bccf1 100644 --- a/dbus/dbus-server-unix.c +++ b/dbus/dbus-server-unix.c @@ -99,7 +99,7 @@ handle_new_client_fd_and_unlock (DBusServer *server, return TRUE; } - transport = _dbus_transport_new_for_fd (client_fd, TRUE, NULL); + transport = _dbus_transport_new_for_fd (client_fd, &server->guid_hex, NULL); if (transport == NULL) { close (client_fd); diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c index 20fb5f3c..0a3b522d 100644 --- a/dbus/dbus-server.c +++ b/dbus/dbus-server.c @@ -75,33 +75,30 @@ init_guid (DBusGUID *guid) */ static char* copy_address_with_guid_appended (const DBusString *address, - const DBusGUID *guid) + const DBusString *guid_hex) { DBusString with_guid; - DBusString guid_str; char *retval; if (!_dbus_string_init (&with_guid)) return NULL; - _dbus_string_init_const_len (&guid_str, guid->as_bytes, - sizeof (guid->as_bytes)); - - if (!_dbus_string_copy (address, 0, &with_guid, 0) || + if (!_dbus_string_copy (address, 0, &with_guid, + _dbus_string_get_length (&with_guid)) || !_dbus_string_append (&with_guid, ",guid=") || - !_dbus_string_hex_encode (&guid_str, 0, - &with_guid, _dbus_string_get_length (&with_guid))) + !_dbus_string_copy (guid_hex, 0, + &with_guid, _dbus_string_get_length (&with_guid))) { _dbus_string_free (&with_guid); return NULL; } retval = NULL; - _dbus_string_copy_data (&with_guid, &retval); + _dbus_string_steal_data (&with_guid, &retval); _dbus_string_free (&with_guid); - return retval; /* may be NULL if copy failed */ + return retval; /* may be NULL if steal_data failed */ } /** @@ -117,7 +114,9 @@ dbus_bool_t _dbus_server_init_base (DBusServer *server, const DBusServerVTable *vtable, const DBusString *address) -{ +{ + DBusString guid_raw; + server->vtable = vtable; server->refcount.value = 1; @@ -125,10 +124,20 @@ _dbus_server_init_base (DBusServer *server, server->watches = NULL; server->timeouts = NULL; + if (!_dbus_string_init (&server->guid_hex)) + return FALSE; + init_guid (&server->guid); + _dbus_string_init_const_len (&guid_raw, server->guid.as_bytes, + sizeof (server->guid.as_bytes)); + if (!_dbus_string_hex_encode (&guid_raw, 0, + &server->guid_hex, + _dbus_string_get_length (&server->guid_hex))) + goto failed; + server->address = copy_address_with_guid_appended (address, - &server->guid); + &server->guid_hex); if (server->address == NULL) goto failed; @@ -171,6 +180,7 @@ _dbus_server_init_base (DBusServer *server, dbus_free (server->address); server->address = NULL; } + _dbus_string_free (&server->guid_hex); return FALSE; } @@ -205,6 +215,8 @@ _dbus_server_finalize_base (DBusServer *server) dbus_free (server->address); dbus_free_string_array (server->auth_mechanisms); + + _dbus_string_free (&server->guid_hex); } diff --git a/dbus/dbus-threads.c b/dbus/dbus-threads.c index 0d004d62..fc83682d 100644 --- a/dbus/dbus-threads.c +++ b/dbus/dbus-threads.c @@ -231,7 +231,8 @@ init_global_locks (void) LOCK_ADDR (bus), LOCK_ADDR (shutdown_funcs), LOCK_ADDR (system_users), - LOCK_ADDR (message_cache) + LOCK_ADDR (message_cache), + LOCK_ADDR (shared_connections) #undef LOCK_ADDR }; diff --git a/dbus/dbus-transport-protected.h b/dbus/dbus-transport-protected.h index 54195933..879c3db8 100644 --- a/dbus/dbus-transport-protected.h +++ b/dbus/dbus-transport-protected.h @@ -113,7 +113,7 @@ struct DBusTransport dbus_bool_t _dbus_transport_init_base (DBusTransport *transport, const DBusTransportVTable *vtable, - dbus_bool_t server, + const DBusString *server_guid, const DBusString *address); void _dbus_transport_finalize_base (DBusTransport *transport); diff --git a/dbus/dbus-transport-unix.c b/dbus/dbus-transport-unix.c index 2959886a..4c07d5f3 100644 --- a/dbus/dbus-transport-unix.c +++ b/dbus/dbus-transport-unix.c @@ -1114,13 +1114,13 @@ static DBusTransportVTable unix_vtable = { * boil down to a full duplex file descriptor. * * @param fd the file descriptor. - * @param server #TRUE if this transport is on the server side of a connection + * @param server_guid non-#NULL if this transport is on the server side of a connection * @param address the transport's address * @returns the new transport, or #NULL if no memory. */ DBusTransport* _dbus_transport_new_for_fd (int fd, - dbus_bool_t server, + const DBusString *server_guid, const DBusString *address) { DBusTransportUnix *unix_transport; @@ -1151,7 +1151,7 @@ _dbus_transport_new_for_fd (int fd, if (!_dbus_transport_init_base (&unix_transport->base, &unix_vtable, - server, address)) + server_guid, address)) goto failed_4; unix_transport->fd = fd; diff --git a/dbus/dbus-transport-unix.h b/dbus/dbus-transport-unix.h index 254144d9..73986ba6 100644 --- a/dbus/dbus-transport-unix.h +++ b/dbus/dbus-transport-unix.h @@ -28,7 +28,7 @@ DBUS_BEGIN_DECLS DBusTransport* _dbus_transport_new_for_fd (int fd, - dbus_bool_t server, + const DBusString *server_guid, const DBusString *address); DBusTransport* _dbus_transport_new_for_domain_socket (const char *path, dbus_bool_t abstract, diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index c08573d8..b271d944 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -75,19 +75,22 @@ live_messages_size_notify (DBusCounter *counter, } /** - * Initializes the base class members of DBusTransport. - * Chained up to by subclasses in their constructor. + * Initializes the base class members of DBusTransport. Chained up to + * by subclasses in their constructor. The server GUID is the + * globally unique ID for the server creating this connection + * and will be #NULL for the client side of a connection. The GUID + * is in hex format. * * @param transport the transport being created. * @param vtable the subclass vtable. - * @param server #TRUE if this transport is on the server side of a connection + * @param server_guid non-#NULL if this transport is on the server side of a connection * @param address the address of the transport * @returns #TRUE on success. */ dbus_bool_t _dbus_transport_init_base (DBusTransport *transport, const DBusTransportVTable *vtable, - dbus_bool_t server, + const DBusString *server_guid, const DBusString *address) { DBusMessageLoader *loader; @@ -99,8 +102,8 @@ _dbus_transport_init_base (DBusTransport *transport, if (loader == NULL) return FALSE; - if (server) - auth = _dbus_auth_server_new (); + if (server_guid) + auth = _dbus_auth_server_new (server_guid); else auth = _dbus_auth_client_new (); if (auth == NULL) @@ -117,7 +120,7 @@ _dbus_transport_init_base (DBusTransport *transport, return FALSE; } - if (server) + if (server_guid) { _dbus_assert (address == NULL); address_copy = NULL; @@ -142,9 +145,9 @@ _dbus_transport_init_base (DBusTransport *transport, transport->live_messages_size = counter; transport->authenticated = FALSE; transport->disconnected = FALSE; - transport->send_credentials_pending = !server; - transport->receive_credentials_pending = server; - transport->is_server = server; + transport->is_server = (server_guid != NULL); + transport->send_credentials_pending = !transport->is_server; + transport->receive_credentials_pending = transport->is_server; transport->address = address_copy; transport->unix_user_function = NULL; @@ -195,33 +198,22 @@ _dbus_transport_finalize_base (DBusTransport *transport) } /** - * Opens a new transport for the given address. (This opens a - * client-side-of-the-connection transport.) - * - * @todo error messages on bad address could really be better. - * DBusResultCode is a bit limiting here. + * Try to open a new transport for the given address entry. (This + * opens a client-side-of-the-connection transport.) * - * @param address the address. + * @param entry the address entry * @param error location to store reason for failure. * @returns new transport of #NULL on failure. */ DBusTransport* -_dbus_transport_open (const char *address, - DBusError *error) +_dbus_transport_open (DBusAddressEntry *entry, + DBusError *error) { DBusTransport *transport; - DBusAddressEntry **entries; - DBusError tmp_error; - DBusError first_error; - int len, i; const char *address_problem_type; const char *address_problem_field; const char *address_problem_other; - - _DBUS_ASSERT_ERROR_IS_CLEAR (error); - - if (!dbus_parse_address (address, &entries, &len, error)) - return NULL; + const char *method; _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -229,125 +221,96 @@ _dbus_transport_open (const char *address, address_problem_type = NULL; address_problem_field = NULL; address_problem_other = NULL; - - dbus_error_init (&tmp_error); - dbus_error_init (&first_error); - for (i = 0; i < len; i++) - { - const char *method; - - method = dbus_address_entry_get_method (entries[i]); + + method = dbus_address_entry_get_method (entry); + _dbus_assert (method != NULL); - if (strcmp (method, "unix") == 0) - { - const char *path = dbus_address_entry_get_value (entries[i], "path"); - const char *tmpdir = dbus_address_entry_get_value (entries[i], "tmpdir"); - const char *abstract = dbus_address_entry_get_value (entries[i], "abstract"); + if (strcmp (method, "unix") == 0) + { + const char *path = dbus_address_entry_get_value (entry, "path"); + const char *tmpdir = dbus_address_entry_get_value (entry, "tmpdir"); + const char *abstract = dbus_address_entry_get_value (entry, "abstract"); - if (tmpdir != NULL) - { - address_problem_other = "cannot use the \"tmpdir\" option for an address to connect to, only in an address to listen on"; - goto bad_address; - } + if (tmpdir != NULL) + { + address_problem_other = "cannot use the \"tmpdir\" option for an address to connect to, only in an address to listen on"; + goto bad_address; + } - if (path == NULL && abstract == NULL) - { - address_problem_type = "unix"; - address_problem_field = "path or abstract"; - goto bad_address; - } + if (path == NULL && abstract == NULL) + { + address_problem_type = "unix"; + address_problem_field = "path or abstract"; + goto bad_address; + } - if (path != NULL && abstract != NULL) - { - address_problem_other = "can't specify both \"path\" and \"abstract\" options in an address"; - goto bad_address; - } + if (path != NULL && abstract != NULL) + { + address_problem_other = "can't specify both \"path\" and \"abstract\" options in an address"; + goto bad_address; + } - if (path) - transport = _dbus_transport_new_for_domain_socket (path, FALSE, - &tmp_error); - else - transport = _dbus_transport_new_for_domain_socket (abstract, TRUE, - &tmp_error); - } - else if (strcmp (method, "tcp") == 0) - { - const char *host = dbus_address_entry_get_value (entries[i], "host"); - const char *port = dbus_address_entry_get_value (entries[i], "port"); - DBusString str; - long lport; - dbus_bool_t sresult; + if (path) + transport = _dbus_transport_new_for_domain_socket (path, FALSE, + error); + else + transport = _dbus_transport_new_for_domain_socket (abstract, TRUE, + error); + } + else if (strcmp (method, "tcp") == 0) + { + const char *host = dbus_address_entry_get_value (entry, "host"); + const char *port = dbus_address_entry_get_value (entry, "port"); + DBusString str; + long lport; + dbus_bool_t sresult; - if (port == NULL) - { - address_problem_type = "tcp"; - address_problem_field = "port"; - goto bad_address; - } + if (port == NULL) + { + address_problem_type = "tcp"; + address_problem_field = "port"; + goto bad_address; + } - _dbus_string_init_const (&str, port); - sresult = _dbus_string_parse_int (&str, 0, &lport, NULL); - _dbus_string_free (&str); + _dbus_string_init_const (&str, port); + sresult = _dbus_string_parse_int (&str, 0, &lport, NULL); + _dbus_string_free (&str); - if (sresult == FALSE || lport <= 0 || lport > 65535) - { - address_problem_other = "Port is not an integer between 0 and 65535"; - goto bad_address; - } + if (sresult == FALSE || lport <= 0 || lport > 65535) + { + address_problem_other = "Port is not an integer between 0 and 65535"; + goto bad_address; + } - transport = _dbus_transport_new_for_tcp_socket (host, lport, &tmp_error); - } + transport = _dbus_transport_new_for_tcp_socket (host, lport, error); + } #ifdef DBUS_BUILD_TESTS - else if (strcmp (method, "debug-pipe") == 0) - { - const char *name = dbus_address_entry_get_value (entries[i], "name"); + else if (strcmp (method, "debug-pipe") == 0) + { + const char *name = dbus_address_entry_get_value (entry, "name"); - if (name == NULL) - { - address_problem_type = "debug-pipe"; - address_problem_field = "name"; - goto bad_address; - } - - transport = _dbus_transport_debug_pipe_new (name, &tmp_error); - } -#endif - else + if (name == NULL) { - address_problem_other = "Unknown address type (examples of valid types are \"unix\" and \"tcp\")"; + address_problem_type = "debug-pipe"; + address_problem_field = "name"; goto bad_address; } - - if (transport) - break; - - _DBUS_ASSERT_ERROR_IS_SET (&tmp_error); - - if (i == 0) - dbus_move_error (&tmp_error, &first_error); - else - dbus_error_free (&tmp_error); - } - - _DBUS_ASSERT_ERROR_IS_CLEAR (error); - _DBUS_ASSERT_ERROR_IS_CLEAR (&tmp_error); - - if (transport == NULL) - { - _DBUS_ASSERT_ERROR_IS_SET (&first_error); - dbus_move_error (&first_error, error); + + transport = _dbus_transport_debug_pipe_new (name, error); } +#endif else { - dbus_error_free (&first_error); + address_problem_other = "Unknown address type (examples of valid types are \"unix\" and \"tcp\")"; + goto bad_address; } + + if (transport == NULL) + _DBUS_ASSERT_ERROR_IS_SET (error); - dbus_address_entries_free (entries); return transport; - + bad_address: - dbus_address_entries_free (entries); - if (address_problem_type != NULL) dbus_set_error (error, DBUS_ERROR_BAD_ADDRESS, "Address of type %s was missing argument %s", diff --git a/dbus/dbus-transport.h b/dbus/dbus-transport.h index 8359474d..502cfb05 100644 --- a/dbus/dbus-transport.h +++ b/dbus/dbus-transport.h @@ -26,12 +26,13 @@ #include #include #include +#include DBUS_BEGIN_DECLS typedef struct DBusTransport DBusTransport; -DBusTransport* _dbus_transport_open (const char *address, +DBusTransport* _dbus_transport_open (DBusAddressEntry *entry, DBusError *error); DBusTransport* _dbus_transport_ref (DBusTransport *transport); void _dbus_transport_unref (DBusTransport *transport); diff --git a/doc/TODO b/doc/TODO index 77d595a9..dbe2d251 100644 --- a/doc/TODO +++ b/doc/TODO @@ -33,21 +33,9 @@ Important for 1.0 - dbus-pending-call.c has some API and thread safety issues to review - - make dbus_connection_open() return a shared connection from a pool. - Add dbus_connection_open_private() that works like the current one. - To do this, each DBusServer could have a 128-bit GUID. This GUID - would be in the address from dbus_server_get_address(). On - connection to a server, the GUID would be provided as the first - thing in the auth protocol, and verified vs. the expected GUID if a - GUID was in the address used to connect. A hash from GUID to - connection would be kept, so attempts to connect to a GUID already - in the hash would return a shared existing connection. - - The purpose of all this is to allow a dbus_g_proxy_to_string() that - would convert the proxy to an "IOR" and dbus_g_proxy_from_string() - that would decode; using these, dbus-glib users could avoid - DBusConnection entirely. Of course the same applies to other kinds - of binding. + - transmit GUID from server to client in the auth protocol, so + connections can be shared even if the address used to connect + to them didn't have a GUID in it. Important for 1.0 GLib Bindings === @@ -74,6 +62,13 @@ Might as Well for 1.0 Can Be Post 1.0 === + - Allow a dbus_g_proxy_to_string()/g_object_to_string() that + would convert the proxy to an "IOR" and dbus_g_proxy_from_string() + that would decode; using these, dbus-glib users could avoid + DBusConnection entirely. Of course the same applies to other kinds + of binding. This would use dbus_connection_open()'s connection-sharing + feature to avoid massive proliferation of connections. + - DBusWatchList/TimeoutList duplicate a lot of code, as do protected_change_watch/protected_change_timeout in dbus-connection.c and dbus-server.c. This could all be mopped up, cut-and-paste diff --git a/test/glib/test-profile.c b/test/glib/test-profile.c index ca4f33c0..f23c7e26 100644 --- a/test/glib/test-profile.c +++ b/test/glib/test-profile.c @@ -190,7 +190,7 @@ no_bus_thread_func (void *data) g_printerr ("Starting client thread %p\n", g_thread_self()); dbus_error_init (&error); - connection = dbus_connection_open (messages_address, &error); + connection = dbus_connection_open_private (messages_address, &error); if (connection == NULL) { g_printerr ("could not open connection: %s\n", error.message); @@ -361,7 +361,7 @@ with_bus_thread_func (void *data) } dbus_error_init (&error); - connection = dbus_connection_open (address, &error); + connection = dbus_connection_open_private (address, &error); if (connection == NULL) { g_printerr ("could not open connection to bus: %s\n", error.message); -- cgit