summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHavoc Pennington <hp@redhat.com>2006-10-01 17:21:03 +0000
committerHavoc Pennington <hp@redhat.com>2006-10-01 17:21:03 +0000
commit7020b573764bb86551d329e867c2e87172424c9b (patch)
tree3b36225877c0d2d57bef58b079955b9f0623f96a
parenteb1e11babd60dc618753aaceec14821526c96a14 (diff)
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.
-rw-r--r--ChangeLog15
-rw-r--r--dbus/dbus-bus.c9
-rw-r--r--dbus/dbus-connection.c145
-rw-r--r--dbus/dbus-sysdeps.c2
-rw-r--r--test/test-service.c11
-rw-r--r--test/test-shell-service.c1
6 files changed, 121 insertions, 62 deletions
diff --git a/ChangeLog b/ChangeLog
index 4139958e..ed468009 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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;
}