dbus/dbus Makefile.am, 1.84, 1.85 dbus-bus.c, 1.58, 1.59 dbus-bus.h,
1.18, 1.19 dbus-connection-internal.h, 1.28,
1.29 dbus-connection.c, 1.135, 1.136 dbus-internals.c, 1.49,
1.50 dbus-server-debug-pipe.c, 1.23, 1.24 dbus-server-socket.c,
1.2, 1.3 dbus-server.c, 1.49, 1.50 dbus-sysdeps-unix.c, 1.7,
1.8 dbus-sysdeps.c, 1.114, 1.115
Havoc Pennington
hp at kemper.freedesktop.org
Sun Oct 1 08:36:21 PDT 2006
Update of /cvs/dbus/dbus/dbus
In directory kemper:/tmp/cvs-serv11502/dbus
Modified Files:
Makefile.am dbus-bus.c dbus-bus.h dbus-connection-internal.h
dbus-connection.c dbus-internals.c dbus-server-debug-pipe.c
dbus-server-socket.c dbus-server.c dbus-sysdeps-unix.c
dbus-sysdeps.c
Log Message:
2006-10-01 Havoc Pennington <hp at redhat.com>
* dbus/dbus-connection.c (_dbus_connection_close_if_only_one_ref):
Add a hack to make DBusNewConnectionFunction work right.
* dbus/dbus-server-socket.c (handle_new_client_fd_and_unlock): use
the hack here. Also, fix the todo about refcount leak.
* dbus/dbus-server-debug-pipe.c (_dbus_transport_debug_pipe_new):
and use the hack here
* dbus/dbus-connection.c: Kill the "shared" flag vs. the
"shareable" flag; this was completely broken, since it meant
dbus_connection_open() returned a connection of unknown
shared-ness. Now, we always hold a ref on anything opened
as shareable.
Move the call to notify dbus-bus.c into
connection_forget_shared_unlocked, so libdbus consistently forgets
all its knowledge of a connection at once. This exposed numerous
places where things were totally broken if we dropped a ref inside
get_dispatch_status_unlocked where
connection_forget_shared_unlocked was previously, so move
connection_forget_shared_unlocked into
_dbus_connection_update_dispatch_status_and_unlock. Also move the
exit_on_disconnect here.
(shared_connections_shutdown): this assumed weak refs to the
shared connections; since we have strong refs now, the assertion
was failing and stuff was left in the hash. Fix it to close
still-open shared connections.
* bus/dispatch.c: fixup to use dbus_connection_open_private on the
debug pipe connections
* dbus/dbus-connection.c (dbus_connection_dispatch): only notify
dbus-bus.c if the closed connection is in fact shared
(_dbus_connection_close_possibly_shared): rename from
_dbus_connection_close_internal
(dbus_connection_close, dbus_connection_open,
dbus_connection_open_private): Improve docs to explain the deal
with when you should close or unref or both
* dbus/dbus-bus.c
(_dbus_bus_notify_shared_connection_disconnected_unlocked): rename
from _dbus_bus_check_connection_and_unref_unlocked and modify to
loop over all connections
* test/test-utils.c (test_connection_shutdown): don't try to close
shared connections.
* test/name-test/test-threads-init.c (main): fix warnings in here
* dbus/dbus-sysdeps.c (_dbus_abort): support DBUS_BLOCK_ON_ABORT
env variable to cause blocking waiting for gdb; drop
DBUS_PRINT_BACKTRACE and just call _dbus_print_backtrace()
unconditionally.
* configure.in: add -export-dynamic to libtool flags if assertions enabled
so _dbus_print_backtrace works.
* dbus/dbus-sysdeps-unix.c (_dbus_print_backtrace): use fprintf
instead of _dbus_verbose to print the backtrace, and diagnose lack
of -rdynamic/-export-dynamic
Index: Makefile.am
===================================================================
RCS file: /cvs/dbus/dbus/dbus/Makefile.am,v
retrieving revision 1.84
retrieving revision 1.85
diff -u -d -r1.84 -r1.85
--- Makefile.am 1 Oct 2006 03:18:47 -0000 1.84
+++ Makefile.am 1 Oct 2006 15:36:18 -0000 1.85
@@ -164,7 +164,9 @@
libdbus_1_la_LIBADD= $(DBUS_CLIENT_LIBS)
## don't export symbols that start with "_" (we use this
## convention for internal symbols)
-libdbus_1_la_LDFLAGS= -export-symbols-regex "^[^_].*" -version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) -no-undefined
+libdbus_1_la_LDFLAGS= -export-symbols-regex "^[^_].*" -version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) -no-undefined @R_DYNAMIC_LDFLAG@
+
+libdbus_convenience_la_LDFLAGS=@R_DYNAMIC_LDFLAG@
## note that TESTS has special meaning (stuff to use in make check)
## so if adding tests not to be run in make check, don't add them to
@@ -184,6 +186,7 @@
dbus-test-main.c
dbus_test_LDADD=libdbus-convenience.la
+dbus_test_LDFLAGS=@R_DYNAMIC_LDFLAG@
## mop up the gcov files
clean-local:
Index: dbus-bus.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-bus.c,v
retrieving revision 1.58
retrieving revision 1.59
diff -u -d -r1.58 -r1.59
--- dbus-bus.c 1 Oct 2006 03:34:21 -0000 1.58
+++ dbus-bus.c 1 Oct 2006 15:36:18 -0000 1.59
@@ -158,6 +158,7 @@
/* Use default system bus address if none set in environment */
bus_connection_addresses[DBUS_BUS_SYSTEM] =
_dbus_strdup (DBUS_SYSTEM_BUS_DEFAULT_ADDRESS);
+
if (bus_connection_addresses[DBUS_BUS_SYSTEM] == NULL)
return FALSE;
@@ -179,7 +180,8 @@
if (bus_connection_addresses[DBUS_BUS_SESSION] == NULL)
bus_connection_addresses[DBUS_BUS_SESSION] =
_dbus_strdup (DBUS_SESSION_BUS_DEFAULT_ADDRESS);
- if (bus_connection_addresses[DBUS_BUS_SESSION] == NULL)
+
+ if (bus_connection_addresses[DBUS_BUS_SESSION] == NULL)
return FALSE;
_dbus_verbose (" \"%s\"\n", bus_connection_addresses[DBUS_BUS_SESSION] ?
@@ -310,27 +312,32 @@
return bd;
}
-/* internal function that checks to see if this
- is a shared connection owned by the bus and if it is unref it */
+/**
+ * Internal function that checks to see if this
+ * is a shared connection owned by the bus and if it is unref it.
+ *
+ * @param connection a connection that has been disconnected.
+ */
void
-_dbus_bus_check_connection_and_unref_unlocked (DBusConnection *connection)
+_dbus_bus_notify_shared_connection_disconnected_unlocked (DBusConnection *connection)
{
+ int i;
+
_DBUS_LOCK (bus);
- if (bus_connections[DBUS_BUS_SYSTEM] == connection)
- {
- bus_connections[DBUS_BUS_SYSTEM] = NULL;
- _dbus_connection_unref_unlocked (connection);
- }
- else if (bus_connections[DBUS_BUS_SESSION] == connection)
- {
- bus_connections[DBUS_BUS_SESSION] = NULL;
- _dbus_connection_unref_unlocked (connection);
- }
- else if (bus_connections[DBUS_BUS_STARTER] == connection)
+ /* We are expecting to have the connection saved in only one of these
+ * slots, but someone could in a pathological case set system and session
+ * bus to the same bus or something. Or set one of them to the starter
+ * bus without setting the starter bus type in the env variable.
+ * So we don't break the loop as soon as we find a match.
+ */
+ for (i = 0; i < N_BUS_TYPES; ++i)
{
- bus_connections[DBUS_BUS_STARTER] = NULL;
- _dbus_connection_unref_unlocked (connection);
+ if (bus_connections[i] == connection)
+ {
+ bus_connections[i] = NULL;
+ _dbus_connection_unref_unlocked (connection);
+ }
}
_DBUS_UNLOCK (bus);
@@ -392,7 +399,7 @@
}
if (private)
- connection = dbus_connection_open_private(address, error);
+ connection = dbus_connection_open_private (address, error);
else
connection = dbus_connection_open (address, error);
@@ -412,7 +419,7 @@
if (!dbus_bus_register (connection, error))
{
_DBUS_ASSERT_ERROR_IS_SET (error);
- _dbus_connection_close_internal (connection);
+ _dbus_connection_close_possibly_shared (connection);
dbus_connection_unref (connection);
_DBUS_UNLOCK (bus);
@@ -432,6 +439,8 @@
bd->is_well_known = TRUE;
_DBUS_UNLOCK (bus);
+
+ /* Return a reference to the caller */
return connection;
}
@@ -446,11 +455,22 @@
/**
* Connects to a bus daemon and registers the client with it. If a
* connection to the bus already exists, then that connection is
- * returned. Caller owns a reference to the bus.
+ * returned. The caller of this function owns a reference to the bus.
+ *
+ * The caller may NOT call dbus_connection_close() on this connection;
+ * see dbus_connection_open() and dbus_connection_close() for details
+ * on that.
*
+ * If this function obtains a new connection object never before
+ * returned from dbus_bus_get(), it will call
+ * dbus_connection_set_exit_on_disconnect(), so the application
+ * will exit if the connection closes. You can undo this
+ * by calling dbus_connection_set_exit_on_disconnect() yourself
+ * after you get the connection.
+ *
* @param type bus type
* @param error address where an error can be returned.
- * @returns a DBusConnection with new ref
+ * @returns a #DBusConnection with new ref
*/
DBusConnection *
dbus_bus_get (DBusBusType type,
@@ -460,10 +480,20 @@
}
/**
- * Connects to a bus daemon and registers the client with it. Unlike
- * dbus_bus_get(), always creates a new connection. This connection
+ * Connects to a bus daemon and registers the client with it as with dbus_bus_register().
+ * Unlike dbus_bus_get(), always creates a new connection. This connection
* will not be saved or recycled by libdbus. Caller owns a reference
- * to the bus.
+ * to the bus and must either close it or know it to be closed
+ * prior to releasing this reference.
+ *
+ * See dbus_connection_open_private() for more details on when to
+ * close and unref this connection.
+ *
+ * This function calls
+ * dbus_connection_set_exit_on_disconnect() on the new connection, so the application
+ * will exit if the connection closes. You can undo this
+ * by calling dbus_connection_set_exit_on_disconnect() yourself
+ * after you get the connection.
*
* @param type bus type
* @param error address where an error can be returned.
@@ -481,6 +511,9 @@
* thing an application does when connecting to the message bus.
* If registration succeeds, the unique name will be set,
* and can be obtained using dbus_bus_get_unique_name().
+ *
+ * If you use dbus_bus_get() or dbus_bus_get_private() this
+ * function will be called for you.
*
* @param connection the connection
* @param error place to store errors
Index: dbus-bus.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-bus.h,v
retrieving revision 1.18
retrieving revision 1.19
diff -u -d -r1.18 -r1.19
--- dbus-bus.h 6 Sep 2006 22:00:07 -0000 1.18
+++ dbus-bus.h 1 Oct 2006 15:36:18 -0000 1.19
@@ -68,7 +68,7 @@
const char *rule,
DBusError *error);
-void _dbus_bus_check_connection_and_unref_unlocked (DBusConnection *connection);
+void _dbus_bus_notify_shared_connection_disconnected_unlocked (DBusConnection *connection);
DBUS_END_DECLS
Index: dbus-connection-internal.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection-internal.h,v
retrieving revision 1.28
retrieving revision 1.29
diff -u -d -r1.28 -r1.29
--- dbus-connection-internal.h 7 Sep 2006 19:02:22 -0000 1.28
+++ dbus-connection-internal.h 1 Oct 2006 15:36:18 -0000 1.29
@@ -77,7 +77,8 @@
void _dbus_connection_do_iteration_unlocked (DBusConnection *connection,
unsigned int flags,
int timeout_milliseconds);
-void _dbus_connection_close_internal (DBusConnection *connection);
+void _dbus_connection_close_possibly_shared (DBusConnection *connection);
+void _dbus_connection_close_if_only_one_ref (DBusConnection *connection);
DBusPendingCall* _dbus_pending_call_new (DBusConnection *connection,
int timeout_milliseconds,
Index: dbus-connection.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection.c,v
retrieving revision 1.135
retrieving revision 1.136
diff -u -d -r1.135 -r1.136
--- dbus-connection.c 1 Oct 2006 03:18:47 -0000 1.135
+++ dbus-connection.c 1 Oct 2006 15:36:18 -0000 1.136
@@ -238,9 +238,7 @@
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 shareable : 1; /**< #TRUE if libdbus owns a reference to the connection and can return it from dbus_connection_open() more than once */
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) */
@@ -248,6 +246,14 @@
unsigned int exit_on_disconnect : 1; /**< If #TRUE, exit after handling disconnect signal */
unsigned int route_peer_messages : 1; /**< If #TRUE, if org.freedesktop.DBus.Peer messages have a bus name, don't handle them automatically */
+
+ unsigned int disconnected_message_arrived : 1; /**< We popped or are dispatching the disconnected message.
+ * if the disconnect_message_link is NULL then we queued it, but
+ * this flag is whether it got to the head of the queue.
+ */
+ unsigned int disconnected_message_processed : 1; /**< We did our default handling of the disconnected message,
+ * such as closing the connection.
+ */
#ifndef DBUS_DISABLE_CHECKS
unsigned int have_connection_lock : 1; /**< Used to check locking */
@@ -265,6 +271,8 @@
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 void _dbus_connection_close_possibly_shared_and_unlock (DBusConnection *connection);
+static dbus_bool_t _dbus_connection_get_is_connected_unlocked (DBusConnection *connection);
static DBusMessageFilter *
_dbus_message_filter_ref (DBusMessageFilter *filter)
@@ -366,11 +374,11 @@
*/
void
_dbus_connection_test_get_locks (DBusConnection *conn,
- DBusMutex **mutex_loc,
- DBusMutex **dispatch_mutex_loc,
- DBusMutex **io_path_mutex_loc,
- DBusCondVar **dispatch_cond_loc,
- DBusCondVar **io_path_cond_loc)
+ DBusMutex **mutex_loc,
+ DBusMutex **dispatch_mutex_loc,
+ DBusMutex **io_path_mutex_loc,
+ DBusCondVar **dispatch_cond_loc,
+ DBusCondVar **io_path_cond_loc)
{
*mutex_loc = conn->mutex;
*dispatch_mutex_loc = conn->dispatch_mutex;
@@ -1178,6 +1186,8 @@
connection->exit_on_disconnect = FALSE;
connection->shareable = FALSE;
connection->route_peer_messages = FALSE;
+ connection->disconnected_message_arrived = FALSE;
+ connection->disconnected_message_processed = FALSE;
#ifndef DBUS_DISABLE_CHECKS
connection->generation = _dbus_current_generation;
@@ -1365,10 +1375,42 @@
static void
shared_connections_shutdown (void *data)
{
+ int n_entries;
+
_DBUS_LOCK (shared_connections);
+
+ /* This is a little bit unpleasant... better ideas? */
+ while ((n_entries = _dbus_hash_table_get_n_entries (shared_connections)) > 0)
+ {
+ DBusConnection *connection;
+ DBusMessage *message;
+ DBusHashIter iter;
+
+ _dbus_hash_iter_init (shared_connections, &iter);
+ _dbus_hash_iter_next (&iter);
+
+ connection = _dbus_hash_iter_get_value (&iter);
- _dbus_assert (_dbus_hash_table_get_n_entries (shared_connections) == 0);
+ _DBUS_UNLOCK (shared_connections);
+ dbus_connection_ref (connection);
+ _dbus_connection_close_possibly_shared (connection);
+
+ /* Churn through to the Disconnected message */
+ while ((message = dbus_connection_pop_message (connection)))
+ {
+ dbus_message_unref (message);
+ }
+ dbus_connection_unref (connection);
+
+ _DBUS_LOCK (shared_connections);
+
+ /* The connection should now be dead and not in our hash ... */
+ _dbus_assert (_dbus_hash_table_get_n_entries (shared_connections) < n_entries);
+ }
+
+ _dbus_assert (_dbus_hash_table_get_n_entries (shared_connections) == 0);
+
_dbus_hash_table_unref (shared_connections);
shared_connections = NULL;
@@ -1419,23 +1461,35 @@
if (guid != NULL)
{
- *result = _dbus_hash_table_lookup_string (shared_connections,
- guid);
+ DBusConnection *connection;
+
+ connection = _dbus_hash_table_lookup_string (shared_connections,
+ guid);
- if (*result)
+ if (connection)
{
- /* 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.
+ /* The DBusConnection can't be finalized without taking
+ * the shared_connections lock to remove it from the
+ * hash. So it's safe to ref the connection here.
+ * However, it may be disconnected if the Disconnected
+ * message hasn't been processed yet, in which case we
+ * want to pretend it isn't in the hash and avoid
+ * returning it.
*/
- dbus_connection_ref (*result);
-
- _dbus_verbose ("looked up existing connection to server guid %s\n",
- guid);
+ CONNECTION_LOCK (connection);
+ if (_dbus_connection_get_is_connected_unlocked (connection))
+ {
+ _dbus_connection_ref_unlocked (connection);
+ *result = connection;
+ _dbus_verbose ("looked up existing connection to server guid %s\n",
+ guid);
+ }
+ else
+ {
+ _dbus_verbose ("looked up existing connection to server guid %s but it was disconnected so ignoring it\n",
+ guid);
+ }
+ CONNECTION_UNLOCK (connection);
}
}
@@ -1451,14 +1505,24 @@
char *guid_key;
char *guid_in_connection;
+ HAVE_LOCK_CHECK (connection);
+ _dbus_assert (connection->server_guid == NULL);
+ _dbus_assert (connection->shareable);
+
+ /* get a hard ref on this connection, even if
+ * we won't in fact store it in the hash, we still
+ * need to hold a ref on it until it's disconnected.
+ */
+ _dbus_connection_ref_unlocked (connection);
+
+ if (guid == NULL)
+ return TRUE; /* don't store in the hash */
+
/* 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;
@@ -1483,10 +1547,6 @@
}
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);
@@ -1502,25 +1562,30 @@
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);
+ if (!connection->shareable)
+ return;
- _DBUS_LOCK (shared_connections);
+ if (connection->server_guid != NULL)
+ {
+ _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);
+ }
- if (!_dbus_hash_table_remove_string (shared_connections,
- connection->server_guid))
- _dbus_assert_not_reached ("connection was not in the shared table");
+ _dbus_bus_notify_shared_connection_disconnected_unlocked (connection);
- dbus_free (connection->server_guid);
- connection->server_guid = NULL;
-
- /* remove the hash ref */
+ /* remove our reference held on all shareable connections */
_dbus_connection_unref_unlocked (connection);
- _DBUS_UNLOCK (shared_connections);
}
static DBusConnection*
@@ -1603,36 +1668,42 @@
{
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))
+ if (connection != NULL)
+ {
+ CONNECTION_LOCK (connection);
+
+ if (shared)
{
- _DBUS_SET_OOM (&tmp_error);
- _dbus_connection_close_internal (connection);
- dbus_connection_unref (connection);
- connection = NULL;
+ const char *guid;
+
+ connection->shareable = TRUE;
+
+ /* guid may be NULL */
+ guid = dbus_address_entry_get_value (entries[i], "guid");
+
+ if (!connection_record_shared_unlocked (connection, guid))
+ {
+ _DBUS_SET_OOM (&tmp_error);
+ _dbus_connection_close_possibly_shared_and_unlock (connection);
+ dbus_connection_unref (connection);
+ connection = NULL;
+ }
+
+ /* Note: as of now the connection is possibly shared
+ * since another thread could have pulled it from the table.
+ * However, we still have it locked so that thread isn't
+ * doing anything more than blocking on the lock.
+ */
}
-
- /* but as of now the connection is possibly shared
- * since another thread could have pulled it from the table
- */
}
}
if (connection)
- break;
+ {
+ HAVE_LOCK_CHECK (connection);
+ break;
+ }
_DBUS_ASSERT_ERROR_IS_SET (&tmp_error);
@@ -1641,8 +1712,6 @@
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);
@@ -1655,6 +1724,8 @@
else
{
dbus_error_free (&first_error);
+
+ CONNECTION_UNLOCK (connection);
}
dbus_address_entries_free (entries);
@@ -1683,6 +1754,10 @@
* reason for the failure in the error parameter. Pass #NULL for the
* error parameter if you aren't interested in the reason for
* failure.
+ *
+ * Because this connection is shared, no user of the connection
+ * may call dbus_connection_close(). However, when you are done with the
+ * connection you should call dbus_connection_unref().
*
* @param address the address.
* @param error address where an error can be returned.
@@ -1713,6 +1788,14 @@
* reason for the failure in the error parameter. Pass #NULL for the
* error parameter if you aren't interested in the reason for
* failure.
+ *
+ * When you are done with this connection, you must
+ * dbus_connection_close() to disconnect it,
+ * and dbus_connection_unref() to free the connection object.
+ *
+ * (The dbus_connection_close() can be skipped if the
+ * connection is already known to be disconnected, for example
+ * if you are inside a handler for the Disconnected signal.)
*
* @param address the address.
* @param error address where an error can be returned.
@@ -1869,12 +1952,19 @@
/**
* Decrements the reference count of a DBusConnection, and finalizes
- * it if the count reaches zero. It is a bug to drop the last reference
- * to a connection that has not been disconnected.
+ * it if the count reaches zero.
*
- * @todo in practice it can be quite tricky to never unref a connection
- * that's still connected; maybe there's some way we could avoid
- * the requirement.
+ * Note: it is a bug to drop the last reference to a connection that
+ * is still connected.
+ *
+ * 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.
+ *
+ * For private connections, the creator of the connection must arrange for
+ * dbus_connection_close() to be called prior to dropping the last reference.
+ * Private connections come from dbus_connection_open_private() or dbus_bus_get_private().
*
* @param connection the connection.
*/
@@ -1912,11 +2002,19 @@
}
static void
-_dbus_connection_close_internal_and_unlock (DBusConnection *connection)
+_dbus_connection_close_possibly_shared_and_unlock (DBusConnection *connection)
{
DBusDispatchStatus status;
+
+ HAVE_LOCK_CHECK (connection);
_dbus_verbose ("Disconnecting %p\n", connection);
+
+ /* We need to ref because update_dispatch_status_and_unlock will unref
+ * the connection if it was shared and libdbus was the only remaining
+ * refcount holder.
+ */
+ _dbus_connection_ref_unlocked (connection);
_dbus_transport_disconnect (connection->transport);
@@ -1925,34 +2023,62 @@
/* this calls out to user code */
_dbus_connection_update_dispatch_status_and_unlock (connection, status);
+
+ /* could also call out to user code */
+ dbus_connection_unref (connection);
}
void
-_dbus_connection_close_internal (DBusConnection *connection)
+_dbus_connection_close_possibly_shared (DBusConnection *connection)
{
_dbus_assert (connection != NULL);
_dbus_assert (connection->generation == _dbus_current_generation);
CONNECTION_LOCK (connection);
- _dbus_connection_close_internal_and_unlock (connection);
+ _dbus_connection_close_possibly_shared_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
- * function does not affect the connection's reference count. It's
- * safe to disconnect a connection more than once; all calls after the
+ * Closes a private connection, so no further data can be sent or received.
+ * This disconnects the transport (such as a socket) underlying the
+ * connection.
+ *
+ * Attempts to send messages after closing a connection are safe, but will result in
+ * error replies generated locally in libdbus.
+ *
+ * This function does not affect the connection's reference count. It's
+ * safe to close a connection more than once; all calls after the
* first do nothing. It's impossible to "reopen" a connection, a
* new connection must be created. This function may result in a call
* to the DBusDispatchStatusFunction set with
* 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.
+ * If a connection is dropped by the remote application, it will
+ * close itself.
+ *
+ * You must close a connection prior to releasing the last reference to
+ * the connection. If you dbus_connection_unref() for the last time
+ * without closing the connection, the results are undefined; it
+ * is a bug in your program and libdbus will try to print a warning.
*
- * @param connection the connection.
+ * You may not close a shared connection. Connections created with
+ * dbus_connection_open() or dbus_bus_get() are shared.
+ * These connections are owned by libdbus, and applications should
+ * only unref them, never close them. Applications can know it is
+ * safe to unref these connections because libdbus will be holding a
+ * reference as long as the connection is open. Thus, either the
+ * connection is closed and it is OK to drop the last reference,
+ * or the connection is open and the app knows it does not have the
+ * last reference.
+ *
+ * Connections created with dbus_connection_open_private() or
+ * dbus_bus_get_private() are not kept track of or referenced by
+ * libdbus. The creator of these connections is responsible for
+ * calling dbus_connection_close() prior to releasing the last
+ * reference, if the connection is not already disconnected.
+ *
+ * @param connection the private (unshared) connection to close
*/
void
dbus_connection_close (DBusConnection *connection)
@@ -1962,15 +2088,52 @@
CONNECTION_LOCK (connection);
- if (connection->shared)
+ if (connection->shareable)
{
CONNECTION_UNLOCK (connection);
- _dbus_warn ("Applications can not close shared connections. Please fix this in your app. Ignoring close request and continuing.");
+ _dbus_warn ("Applications must not close shared connections - see dbus_connection_close() docs. This is a bug in the application.\n");
return;
}
- _dbus_connection_close_internal_and_unlock (connection);
+ _dbus_connection_close_possibly_shared_and_unlock (connection);
+}
+
+/**
+ * Used internally to handle the semantics of dbus_server_set_new_connection_function().
+ * If the new connection function does not ref the connection, we want to close it.
+ *
+ * A bit of a hack, probably the new connection function should have returned a value
+ * for whether to close, or should have had to close the connection itself if it
+ * didn't want it.
+ *
+ * But, this works OK as long as the new connection function doesn't do anything
+ * crazy like keep the connection around without ref'ing it.
+ *
+ * We have to lock the connection across refcount check and close in case
+ * the new connection function spawns a thread that closes and unrefs.
+ * In that case, if the app thread
+ * closes and unrefs first, we'll harmlessly close again; if the app thread
+ * still has the ref, we'll close and then the app will close harmlessly.
+ * If the app unrefs without closing, the app is broken since if the
+ * app refs from the new connection function it is supposed to also close.
+ *
+ * If we didn't atomically check the refcount and close with the lock held
+ * though, we could screw this up.
+ *
+ * @param connection the connection
+ */
+void
+_dbus_connection_close_if_only_one_ref (DBusConnection *connection)
+{
+ CONNECTION_LOCK (connection);
+
+ _dbus_assert (connection->refcount.value > 0);
+
+ if (connection->refcount.value == 1)
+ _dbus_connection_close_possibly_shared_and_unlock (connection);
+ else
+ CONNECTION_UNLOCK (connection);
}
static dbus_bool_t
@@ -1981,11 +2144,14 @@
}
/**
- * Gets whether the connection is currently connected. All
- * connections are connected when they are opened. A connection may
+ * Gets whether the connection is currently open. A connection may
* become disconnected when the remote application closes its end, or
* exits; a connection may also be disconnected with
* dbus_connection_close().
+ *
+ * There are not separate states for "closed" and "disconnected," the two
+ * terms are synonymous. This function should really be called
+ * get_is_open() but for historical reasons is not.
*
* @param connection the connection.
* @returns #TRUE if the connection is still alive.
@@ -2557,7 +2723,7 @@
_dbus_hash_iter_init (connection->pending_replies, &iter);
_dbus_hash_iter_next (&iter);
- pending = (DBusPendingCall *) _dbus_hash_iter_get_value (&iter);
+ pending = _dbus_hash_iter_get_value (&iter);
_dbus_pending_call_ref_unlocked (pending);
_dbus_pending_call_queue_timeout_error_unlocked (pending,
@@ -3116,6 +3282,27 @@
return _dbus_connection_read_write_dispatch(connection, timeout_milliseconds, FALSE);
}
+/* We need to call this anytime we pop the head of the queue, and then
+ * update_dispatch_status_and_unlock needs to be called afterward
+ * which will "process" the disconnected message and set
+ * disconnected_message_processed.
+ */
+static void
+check_disconnected_message_arrived_unlocked (DBusConnection *connection,
+ DBusMessage *head_of_queue)
+{
+ HAVE_LOCK_CHECK (connection);
+
+ /* checking that the link is NULL is an optimization to avoid the is_signal call */
+ if (connection->disconnect_message_link == NULL &&
+ dbus_message_is_signal (head_of_queue,
+ DBUS_INTERFACE_LOCAL,
+ "Disconnected"))
+ {
+ connection->disconnected_message_arrived = TRUE;
+ }
+}
+
/**
* Returns the first-received message from the incoming message queue,
* leaving it in the queue. If the queue is empty, returns #NULL.
@@ -3163,11 +3350,15 @@
message = connection->message_borrowed;
+ check_disconnected_message_arrived_unlocked (connection, message);
+
/* Note that we KEEP the dispatch lock until the message is returned */
if (message == NULL)
_dbus_connection_release_dispatch (connection);
CONNECTION_UNLOCK (connection);
+
+ /* We don't update dispatch status until it's returned or stolen */
return message;
}
@@ -3184,6 +3375,8 @@
dbus_connection_return_message (DBusConnection *connection,
DBusMessage *message)
{
+ DBusDispatchStatus status;
+
_dbus_return_if_fail (connection != NULL);
_dbus_return_if_fail (message != NULL);
_dbus_return_if_fail (message == connection->message_borrowed);
@@ -3195,9 +3388,10 @@
connection->message_borrowed = NULL;
- _dbus_connection_release_dispatch (connection);
-
- CONNECTION_UNLOCK (connection);
+ _dbus_connection_release_dispatch (connection);
+
+ status = _dbus_connection_get_dispatch_status_unlocked (connection);
+ _dbus_connection_update_dispatch_status_and_unlock (connection, status);
}
/**
@@ -3214,6 +3408,7 @@
DBusMessage *message)
{
DBusMessage *pop_message;
+ DBusDispatchStatus status;
_dbus_return_if_fail (connection != NULL);
_dbus_return_if_fail (message != NULL);
@@ -3235,8 +3430,9 @@
connection->message_borrowed = NULL;
_dbus_connection_release_dispatch (connection);
-
- CONNECTION_UNLOCK (connection);
+
+ status = _dbus_connection_get_dispatch_status_unlocked (connection);
+ _dbus_connection_update_dispatch_status_and_unlock (connection, status);
}
/* See dbus_connection_pop_message, but requires the caller to own
@@ -3271,6 +3467,8 @@
dbus_message_get_signature (link->data),
connection, connection->n_incoming);
+ check_disconnected_message_arrived_unlocked (connection, link->data);
+
return link;
}
else
@@ -3375,7 +3573,9 @@
_dbus_verbose ("Returning popped message %p\n", message);
_dbus_connection_release_dispatch (connection);
- CONNECTION_UNLOCK (connection);
+
+ status = _dbus_connection_get_dispatch_status_unlocked (connection);
+ _dbus_connection_update_dispatch_status_and_unlock (connection, status);
return message;
}
@@ -3476,12 +3676,12 @@
{
_dbus_verbose ("Sending disconnect message from %s\n",
_DBUS_FUNCTION_NAME);
-
- connection_forget_shared_unlocked (connection);
-
- /* If we have pending calls queued timeouts on disconnect */
+
+ /* 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.
*/
@@ -3538,6 +3738,27 @@
function = connection->dispatch_status_function;
data = connection->dispatch_status_data;
+ if (connection->disconnected_message_arrived &&
+ !connection->disconnected_message_processed)
+ {
+ connection->disconnected_message_processed = TRUE;
+
+ /* this does an unref, but we have a ref
+ * so we should not run the finalizer here
+ * inside the lock.
+ */
+ connection_forget_shared_unlocked (connection);
+
+ if (connection->exit_on_disconnect)
+ {
+ CONNECTION_UNLOCK (connection);
+
+ _dbus_verbose ("Exiting on Disconnected signal\n");
+ _dbus_exit (1);
+ _dbus_assert_not_reached ("Call to exit() returned");
+ }
+ }
+
/* We drop the lock */
CONNECTION_UNLOCK (connection);
@@ -3981,22 +4202,6 @@
{
_dbus_verbose (" ... done dispatching in %s\n", _DBUS_FUNCTION_NAME);
- if (dbus_message_is_signal (message,
- DBUS_INTERFACE_LOCAL,
- "Disconnected"))
- {
- _dbus_bus_check_connection_and_unref_unlocked (connection);
-
- if (connection->exit_on_disconnect)
- {
- CONNECTION_UNLOCK (connection);
-
- _dbus_verbose ("Exiting on Disconnected signal\n");
- _dbus_exit (1);
- _dbus_assert_not_reached ("Call to exit() returned");
- }
- }
-
_dbus_list_free_link (message_link);
dbus_message_unref (message); /* don't want the message to count in max message limits
* in computing dispatch status below
Index: dbus-internals.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-internals.c,v
retrieving revision 1.49
retrieving revision 1.50
diff -u -d -r1.49 -r1.50
--- dbus-internals.c 1 Oct 2006 03:18:47 -0000 1.49
+++ dbus-internals.c 1 Oct 2006 15:36:18 -0000 1.50
@@ -190,6 +190,9 @@
*/
const char _dbus_no_memory_message[] = "Not enough memory";
+static dbus_bool_t warn_initted = FALSE;
+static dbus_bool_t fatal_warnings = FALSE;
+
/**
* Prints a warning message to stderr.
*
@@ -199,12 +202,27 @@
_dbus_warn (const char *format,
...)
{
- /* FIXME not portable enough? */
va_list args;
+ if (!warn_initted)
+ {
+ const char *s;
+ s = _dbus_getenv ("DBUS_FATAL_WARNINGS");
+ if (s && *s)
+ fatal_warnings = TRUE;
+
+ warn_initted = TRUE;
+ }
+
va_start (args, format);
vfprintf (stderr, format, args);
va_end (args);
+
+ if (fatal_warnings)
+ {
+ fflush (stderr);
+ _dbus_abort ();
+ }
}
#ifdef DBUS_ENABLE_VERBOSE_MODE
Index: dbus-server-debug-pipe.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server-debug-pipe.c,v
retrieving revision 1.23
retrieving revision 1.24
diff -u -d -r1.23 -r1.24
--- dbus-server-debug-pipe.c 16 Sep 2006 19:24:08 -0000 1.23
+++ dbus-server-debug-pipe.c 1 Oct 2006 15:36:18 -0000 1.24
@@ -317,6 +317,7 @@
/* If no one grabbed a reference, the connection will die,
* and the client transport will get an immediate disconnect
*/
+ _dbus_connection_close_if_only_one_ref (connection);
dbus_connection_unref (connection);
return client_transport;
Index: dbus-server-socket.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server-socket.c,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -d -r1.2 -r1.3
--- dbus-server-socket.c 16 Sep 2006 19:24:08 -0000 1.2
+++ dbus-server-socket.c 1 Oct 2006 15:36:18 -0000 1.3
@@ -69,14 +69,6 @@
dbus_free (server);
}
-/**
- * @todo unreffing the connection at the end may cause
- * us to drop the last ref to the connection before
- * disconnecting it. That is invalid.
- *
- * @todo doesn't this leak a server refcount if
- * new_connection_function is NULL?
- */
/* Return value is just for memory, not other failures. */
static dbus_bool_t
handle_new_client_fd_and_unlock (DBusServer *server,
@@ -140,10 +132,11 @@
{
(* new_connection_function) (server, connection,
new_connection_data);
- dbus_server_unref (server);
}
+ dbus_server_unref (server);
/* If no one grabbed a reference, the connection will die. */
+ _dbus_connection_close_if_only_one_ref (connection);
dbus_connection_unref (connection);
return TRUE;
Index: dbus-server.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server.c,v
retrieving revision 1.49
retrieving revision 1.50
diff -u -d -r1.49 -r1.50
--- dbus-server.c 1 Oct 2006 03:18:47 -0000 1.49
+++ dbus-server.c 1 Oct 2006 15:36:18 -0000 1.50
@@ -791,7 +791,15 @@
* function is passed each new connection as the connection is
* created. If the new connection function increments the connection's
* reference count, the connection will stay alive. Otherwise, the
- * connection will be unreferenced and closed.
+ * connection will be unreferenced and closed. The new connection
+ * function may also close the connection itself, which is considered
+ * good form if the connection is not wanted.
+ *
+ * The connection here is private in the sense of
+ * dbus_connection_open_private(), so if the new connection function
+ * keeps a reference it must arrange for the connection to be closed.
+ * i.e. libdbus does not own this connection once the new connection
+ * function takes a reference.
*
* @param server the server.
* @param function a function to handle new connections.
Index: dbus-sysdeps-unix.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-sysdeps-unix.c,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -d -r1.7 -r1.8
--- dbus-sysdeps-unix.c 1 Oct 2006 03:18:47 -0000 1.7
+++ dbus-sysdeps-unix.c 1 Oct 2006 15:36:18 -0000 1.8
@@ -2129,14 +2129,14 @@
#if !defined (DBUS_DISABLE_ASSERT) || defined(DBUS_BUILD_TESTS)
/**
- * On GNU libc systems, print a crude backtrace to the verbose log.
- * On other systems, print "no backtrace support"
- *
+ * On GNU libc systems, print a crude backtrace to stderr. On other
+ * systems, print "no backtrace support" and block for possible gdb
+ * attachment if an appropriate environment variable is set.
*/
void
_dbus_print_backtrace (void)
-{
-#if defined (HAVE_BACKTRACE) && defined (DBUS_ENABLE_VERBOSE_MODE)
+{
+#if defined (HAVE_BACKTRACE) && defined (DBUS_BUILT_R_DYNAMIC)
void *bt[500];
int bt_size;
int i;
@@ -2149,13 +2149,17 @@
i = 0;
while (i < bt_size)
{
- _dbus_verbose (" %s\n", syms[i]);
+ /* don't use dbus_warn since it can _dbus_abort() */
+ fprintf (stderr, " %s\n", syms[i]);
++i;
}
+ fflush (stderr);
free (syms);
+#elif defined (HAVE_BACKTRACE) && ! defined (DBUS_BUILT_R_DYNAMIC)
+ fprintf (stderr, " D-Bus not built with -rdynamic so unable to print a backtrace\n");
#else
- _dbus_verbose (" D-Bus not compiled with backtrace support\n");
+ fprintf (stderr, " D-Bus not compiled with backtrace support so unable to print a backtrace\n");
#endif
}
#endif /* asserts or tests enabled */
Index: dbus-sysdeps.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-sysdeps.c,v
retrieving revision 1.114
retrieving revision 1.115
diff -u -d -r1.114 -r1.115
--- dbus-sysdeps.c 1 Oct 2006 03:18:47 -0000 1.114
+++ dbus-sysdeps.c 1 Oct 2006 15:36:18 -0000 1.115
@@ -60,14 +60,20 @@
void
_dbus_abort (void)
{
-#ifdef DBUS_ENABLE_VERBOSE_MODE
const char *s;
- s = _dbus_getenv ("DBUS_PRINT_BACKTRACE");
+
+ _dbus_print_backtrace ();
+
+ s = _dbus_getenv ("DBUS_BLOCK_ON_ABORT");
if (s && *s)
- _dbus_print_backtrace ();
-#endif
+ {
+ /* 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);
+ }
+
abort ();
- _dbus_exit (1); /* in case someone manages to ignore SIGABRT */
+ _dbus_exit (1); /* in case someone manages to ignore SIGABRT ? */
}
#endif
More information about the dbus-commit
mailing list