* 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
This commit is contained in:
John (J5) Palmieri 2006-09-06 22:00:07 +00:00
parent 61316dd897
commit 1eae184450
9 changed files with 111 additions and 39 deletions

View file

@ -1,3 +1,41 @@
2006-09-06 John (J5) Palmieri <johnp@redhat.com>
* 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 <johnp@redhat.com>
* bus/driver.c:

View file

@ -28,6 +28,7 @@
#include "dbus-message.h"
#include "dbus-marshal-validate.h"
#include "dbus-threads-internal.h"
#include "dbus-connection-internal.h"
#include <string.h>
/**
@ -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);

View file

@ -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

View file

@ -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,

View file

@ -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)
{

View file

@ -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
===

View file

@ -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;
}

View file

@ -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;
}

View file

@ -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,