Commit ce173b29 authored by Havoc Pennington's avatar Havoc Pennington

2003-03-16 Havoc Pennington <hp@pobox.com>

	Oops - test code was only testing failure of around 30 of the
	mallocs in the test path, but it turns out there are 500+
	mallocs. I believe this was due to misguided linking setup such
	that there was one copy of dbus_malloc etc. in the daemon and one
	in the shared lib, and only daemon mallocs were tested. In any
	case, the test case now tests all 500+ mallocs, and doesn't pass
	yet, though there are lots of fixes in this patch.

	* dbus/dbus-connection.c (dbus_connection_dispatch_message): fix
	this so that it doesn't need to allocate memory, since it
	has no way of indicating failure due to OOM (and would be
	annoying if it did).

	* dbus/dbus-list.c (_dbus_list_pop_first_link): new function

	* bus/Makefile.am: rearrange to create two self-contained
	libraries, to avoid having libraries with overlapping symbols.
	that was resulting in weirdness, e.g. I'm pretty sure there
	were two copies of global static variables.

	* dbus/dbus-internals.c: move the malloc debug stuff to
	dbus-memory.c

	* dbus/dbus-list.c (free_link): free list mempool if it becomes
	empty.

	* dbus/dbus-memory.c (_dbus_disable_mem_pools): new function

	* dbus/dbus-address.c (dbus_parse_address): free list nodes
	on failure.

	* bus/dispatch.c (bus_dispatch_add_connection): free
	message_handler_slot when no longer using it, so
	memory leak checkers are happy for the test suite.

	* dbus/dbus-server-debug-pipe.c (debug_finalize): free server name

	* bus/bus.c (new_connection_callback): disconnect in here if
	bus_connections_setup_connection fails.

	* bus/connection.c (bus_connections_unref): fix to free the
	connections
	(bus_connections_setup_connection): if this fails, don't
	disconnect the connection, just be sure there are no side
	effects.

	* dbus/dbus-string.c (undo_alignment): unbreak this

	* dbus/dbus-auth.c (_dbus_auth_unref): free some stuff we were
	leaking
	(_dbus_auth_new): fix the order in which we free strings
	on OOM failure

	* bus/connection.c (bus_connection_disconnected): fix to
	not send ServiceDeleted multiple times in case of memory
	allocation failure

	* dbus/dbus-bus.c (dbus_bus_get_base_service): new function to
	get the base service name
	(dbus_bus_register_client): don't return base service name,
	instead store it on the DBusConnection and have an accessor
	function for it.
	(dbus_bus_register_client): rename dbus_bus_register()

	* bus/dispatch.c (check_hello_message): verify that other
	connections on the bus also got the correct results, not
	just the one sending hello
parent f587ce78
2003-03-16 Havoc Pennington <hp@pobox.com>
Oops - test code was only testing failure of around 30 of the
mallocs in the test path, but it turns out there are 500+
mallocs. I believe this was due to misguided linking setup such
that there was one copy of dbus_malloc etc. in the daemon and one
in the shared lib, and only daemon mallocs were tested. In any
case, the test case now tests all 500+ mallocs, and doesn't pass
yet, though there are lots of fixes in this patch.
* dbus/dbus-connection.c (dbus_connection_dispatch_message): fix
this so that it doesn't need to allocate memory, since it
has no way of indicating failure due to OOM (and would be
annoying if it did).
* dbus/dbus-list.c (_dbus_list_pop_first_link): new function
* bus/Makefile.am: rearrange to create two self-contained
libraries, to avoid having libraries with overlapping symbols.
that was resulting in weirdness, e.g. I'm pretty sure there
were two copies of global static variables.
* dbus/dbus-internals.c: move the malloc debug stuff to
dbus-memory.c
* dbus/dbus-list.c (free_link): free list mempool if it becomes
empty.
* dbus/dbus-memory.c (_dbus_disable_mem_pools): new function
* dbus/dbus-address.c (dbus_parse_address): free list nodes
on failure.
* bus/dispatch.c (bus_dispatch_add_connection): free
message_handler_slot when no longer using it, so
memory leak checkers are happy for the test suite.
* dbus/dbus-server-debug-pipe.c (debug_finalize): free server name
* bus/bus.c (new_connection_callback): disconnect in here if
bus_connections_setup_connection fails.
* bus/connection.c (bus_connections_unref): fix to free the
connections
(bus_connections_setup_connection): if this fails, don't
disconnect the connection, just be sure there are no side
effects.
* dbus/dbus-string.c (undo_alignment): unbreak this
* dbus/dbus-auth.c (_dbus_auth_unref): free some stuff we were
leaking
(_dbus_auth_new): fix the order in which we free strings
on OOM failure
* bus/connection.c (bus_connection_disconnected): fix to
not send ServiceDeleted multiple times in case of memory
allocation failure
* dbus/dbus-bus.c (dbus_bus_get_base_service): new function to
get the base service name
(dbus_bus_register_client): don't return base service name,
instead store it on the DBusConnection and have an accessor
function for it.
(dbus_bus_register_client): rename dbus_bus_register()
* bus/dispatch.c (check_hello_message): verify that other
connections on the bus also got the correct results, not
just the one sending hello
2003-03-15 Havoc Pennington <hp@pobox.com>
Make it pass the Hello handling test including all OOM codepaths.
......
......@@ -6,9 +6,7 @@ EFENCE=
bin_PROGRAMS=dbus-daemon-1
noinst_LTLIBRARIES=libdbus-daemon.la
libdbus_daemon_la_SOURCES= \
BUS_SOURCES= \
activation.c \
activation.h \
bus.c \
......@@ -30,18 +28,14 @@ libdbus_daemon_la_SOURCES= \
utils.c \
utils.h
libdbus_daemon_la_LIBADD= \
$(top_builddir)/dbus/libdbus-convenience.la
dbus_daemon_1_SOURCES= \
$(BUS_SOURCES) \
main.c
dbus_daemon_1_LDADD= \
$(EFENCE) \
$(DBUS_BUS_LIBS) \
$(top_builddir)/bus/libdbus-daemon.la \
$(top_builddir)/dbus/libdbus-1.la
$(top_builddir)/dbus/libdbus-convenience.la
## 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
......@@ -58,9 +52,10 @@ endif
noinst_PROGRAMS=$(TESTS)
bus_test_SOURCES= \
$(BUS_SOURCES) \
test-main.c
bus_test_LDADD= $(top_builddir)/dbus/libdbus-1.la libdbus-daemon.la
bus_test_LDADD=$(top_builddir)/dbus/libdbus-convenience.la
## mop up the gcov files
clean-local:
......
......@@ -94,8 +94,17 @@ new_connection_callback (DBusServer *server,
BusContext *context = data;
if (!bus_connections_setup_connection (context->connections, new_connection))
_dbus_verbose ("No memory to setup new connection\n");
{
_dbus_verbose ("No memory to setup new connection\n");
/* if we don't do this, it will get unref'd without
* being disconnected... kind of strange really
* that we have to do this, people won't get it right
* in general.
*/
dbus_connection_disconnect (new_connection);
}
/* on OOM, we won't have ref'd the connection so it will die. */
}
......@@ -223,16 +232,34 @@ bus_context_unref (BusContext *context)
if (context->refcount == 0)
{
_dbus_verbose ("Finalizing bus context %p\n", context);
bus_context_shutdown (context);
if (context->connections)
{
bus_connections_unref (context->connections);
context->connections = NULL;
}
if (context->registry)
bus_registry_unref (context->registry);
if (context->connections)
bus_connections_unref (context->connections);
{
bus_registry_unref (context->registry);
context->registry = NULL;
}
if (context->activation)
bus_activation_unref (context->activation);
{
bus_activation_unref (context->activation);
context->activation = NULL;
}
if (context->server)
dbus_server_unref (context->server);
{
dbus_server_unref (context->server);
context->server = NULL;
}
dbus_free (context->address);
dbus_free (context);
}
......
......@@ -70,38 +70,38 @@ bus_connection_disconnected (DBusConnection *connection)
* stuff, not just sending a message (so we can e.g. revert
* removal of service owners).
*/
{
BusTransaction *transaction;
DBusError error;
dbus_error_init (&error);
transaction = NULL;
while (transaction == NULL)
{
transaction = bus_transaction_new (d->connections->context);
bus_wait_for_memory ();
}
while ((service = _dbus_list_get_last (&d->services_owned)))
{
retry:
if (!bus_service_remove_owner (service, connection,
transaction, &error))
{
if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
{
dbus_error_free (&error);
bus_wait_for_memory ();
goto retry;
}
else
_dbus_assert_not_reached ("Removing service owner failed for non-memory-related reason");
}
}
bus_transaction_execute_and_free (transaction);
}
while ((service = _dbus_list_get_last (&d->services_owned)))
{
BusTransaction *transaction;
DBusError error;
retry:
dbus_error_init (&error);
transaction = NULL;
while (transaction == NULL)
{
transaction = bus_transaction_new (d->connections->context);
bus_wait_for_memory ();
}
if (!bus_service_remove_owner (service, connection,
transaction, &error))
{
if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
{
dbus_error_free (&error);
bus_transaction_cancel_and_free (transaction);
bus_wait_for_memory ();
goto retry;
}
else
_dbus_assert_not_reached ("Removing service owner failed for non-memory-related reason");
}
bus_transaction_execute_and_free (transaction);
}
bus_dispatch_remove_connection (connection);
......@@ -247,8 +247,17 @@ bus_connections_unref (BusConnections *connections)
connections->refcount -= 1;
if (connections->refcount == 0)
{
/* FIXME free each connection... */
_dbus_assert_not_reached ("shutting down connections not implemented");
while (connections->list != NULL)
{
DBusConnection *connection;
connection = connections->list->data;
dbus_connection_ref (connection);
dbus_connection_disconnect (connection);
bus_connection_disconnected (connection);
dbus_connection_unref (connection);
}
_dbus_list_clear (&connections->list);
......@@ -261,7 +270,8 @@ bus_connections_setup_connection (BusConnections *connections,
DBusConnection *connection)
{
BusConnectionData *d;
dbus_bool_t retval;
d = dbus_new0 (BusConnectionData, 1);
if (d == NULL)
......@@ -277,13 +287,8 @@ bus_connections_setup_connection (BusConnections *connections,
dbus_free (d);
return FALSE;
}
if (!_dbus_list_append (&connections->list, connection))
{
/* this will free our data when connection gets finalized */
dbus_connection_disconnect (connection);
return FALSE;
}
retval = FALSE;
if (!dbus_connection_set_watch_functions (connection,
(DBusAddWatchFunction) add_connection_watch,
......@@ -291,32 +296,46 @@ bus_connections_setup_connection (BusConnections *connections,
NULL,
connection,
NULL))
{
dbus_connection_disconnect (connection);
return FALSE;
}
goto out;
if (!dbus_connection_set_timeout_functions (connection,
(DBusAddTimeoutFunction) add_connection_timeout,
(DBusRemoveTimeoutFunction) remove_connection_timeout,
NULL,
connection, NULL))
{
dbus_connection_disconnect (connection);
return FALSE;
}
goto out;
/* Setup the connection with the dispatcher */
if (!bus_dispatch_add_connection (connection))
goto out;
if (!_dbus_list_append (&connections->list, connection))
{
dbus_connection_disconnect (connection);
return FALSE;
bus_dispatch_remove_connection (connection);
goto out;
}
dbus_connection_ref (connection);
retval = TRUE;
out:
if (!retval)
{
if (!dbus_connection_set_watch_functions (connection,
NULL, NULL, NULL,
connection,
NULL))
_dbus_assert_not_reached ("setting watch functions to NULL failed");
if (!dbus_connection_set_timeout_functions (connection,
NULL, NULL, NULL,
connection,
NULL))
_dbus_assert_not_reached ("setting timeout functions to NULL failed");
}
return TRUE;
return retval;
}
......@@ -607,8 +626,10 @@ bus_transaction_send_message (BusTransaction *transaction,
BusConnectionData *d;
DBusList *link;
_dbus_verbose (" trying to add message %s to transaction\n",
dbus_message_get_name (message));
_dbus_verbose (" trying to add message %s to transaction%s\n",
dbus_message_get_name (message),
dbus_connection_get_is_connected (connection) ?
"" : " (disconnected)");
if (!dbus_connection_get_is_connected (connection))
return TRUE; /* silently ignore disconnected connections */
......@@ -702,6 +723,8 @@ void
bus_transaction_cancel_and_free (BusTransaction *transaction)
{
DBusConnection *connection;
_dbus_verbose ("TRANSACTION: cancelled\n");
while ((connection = _dbus_list_pop_first (&transaction->connections)))
connection_cancel_transaction (connection, transaction);
......@@ -754,6 +777,8 @@ bus_transaction_execute_and_free (BusTransaction *transaction)
* send the messages
*/
DBusConnection *connection;
_dbus_verbose ("TRANSACTION: executing\n");
while ((connection = _dbus_list_pop_first (&transaction->connections)))
connection_execute_transaction (connection, transaction);
......@@ -796,9 +821,6 @@ bus_transaction_send_error_reply (BusTransaction *transaction,
_dbus_assert (error != NULL);
_DBUS_ASSERT_ERROR_IS_SET (error);
_dbus_verbose (" trying to add error %s to transaction\n",
error->name);
reply = dbus_message_new_error_reply (in_reply_to,
error->name,
......
This diff is collapsed.
......@@ -27,11 +27,13 @@
#include "test.h"
#include "loop.h"
#include <dbus/dbus-internals.h>
#include <dbus/dbus-list.h>
/* The "debug client" watch/timeout handlers don't dispatch messages,
* as we manually pull them in order to verify them. This is why they
* are different from the real handlers in connection.c
*/
static DBusList *clients = NULL;
static void
client_watch_callback (DBusWatch *watch,
......@@ -95,17 +97,29 @@ client_disconnect_handler (DBusMessageHandler *handler,
DBusMessage *message,
void *user_data)
{
_dbus_verbose ("Removing client %p in disconnect handler\n",
connection);
_dbus_list_remove (&clients, connection);
dbus_connection_unref (connection);
return DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS;
}
static int handler_slot = -1;
dbus_bool_t
bus_setup_debug_client (DBusConnection *connection)
{
DBusMessageHandler *disconnect_handler;
const char *to_handle[] = { DBUS_MESSAGE_LOCAL_DISCONNECT };
dbus_bool_t retval;
if (handler_slot < 0)
handler_slot = dbus_connection_allocate_data_slot ();
if (handler_slot < 0)
return FALSE;
disconnect_handler = dbus_message_handler_new (client_disconnect_handler,
NULL, NULL);
......@@ -139,24 +153,71 @@ bus_setup_debug_client (DBusConnection *connection)
connection, NULL))
goto out;
if (!_dbus_list_append (&clients, connection))
goto out;
/* Set up handler to be destroyed */
if (!dbus_connection_set_data (connection, handler_slot,
disconnect_handler,
(DBusFreeFunction)
dbus_message_handler_unref))
goto out;
retval = TRUE;
out:
if (!retval)
{
dbus_connection_unregister_handler (connection,
disconnect_handler,
to_handle,
_DBUS_N_ELEMENTS (to_handle));
dbus_message_handler_unref (disconnect_handler); /* unregisters it */
dbus_connection_set_watch_functions (connection,
NULL, NULL, NULL, NULL, NULL);
dbus_connection_set_timeout_functions (connection,
NULL, NULL, NULL, NULL, NULL);
_dbus_list_remove_last (&clients, connection);
}
return retval;
}
void
bus_test_clients_foreach (BusConnectionForeachFunction function,
void *data)
{
DBusList *link;
dbus_message_handler_unref (disconnect_handler);
link = _dbus_list_get_first_link (&clients);
while (link != NULL)
{
DBusConnection *connection = link->data;
DBusList *next = _dbus_list_get_next_link (&clients, link);
if (!(* function) (connection, data))
break;
link = next;
}
}
dbus_bool_t
bus_test_client_listed (DBusConnection *connection)
{
DBusList *link;
return retval;
link = _dbus_list_get_first_link (&clients);
while (link != NULL)
{
DBusConnection *c = link->data;
DBusList *next = _dbus_list_get_next_link (&clients, link);
if (c == connection)
return TRUE;
link = next;
}
return FALSE;
}
#endif
......@@ -30,10 +30,13 @@
#include <dbus/dbus.h>
#include <dbus/dbus-string.h>
#include "connection.h"
dbus_bool_t bus_dispatch_test (const DBusString *test_data_dir);
dbus_bool_t bus_setup_debug_client (DBusConnection *connection);
dbus_bool_t bus_dispatch_test (const DBusString *test_data_dir);
dbus_bool_t bus_setup_debug_client (DBusConnection *connection);
void bus_test_clients_foreach (BusConnectionForeachFunction function,
void *data);
dbus_bool_t bus_test_client_listed (DBusConnection *connection);
#endif
......
......@@ -21,7 +21,7 @@ dbusinclude_HEADERS= \
dbus-threads.h \
dbus-types.h
libdbus_1_la_SOURCES= \
DBUS_SOURCES= \
dbus-address.c \
dbus-auth.c \
dbus-auth.h \
......@@ -67,16 +67,7 @@ libdbus_1_la_SOURCES= \
## dbus-md5.c \
## dbus-md5.h \
## this library is linked into both libdbus and the bus
## itself, but does not export any symbols from libdbus.
## i.e. the point is to contain symbols that we don't
## want the shared lib to export, but we do want the
## message bus to be able to use.
noinst_LTLIBRARIES=libdbus-convenience.la
libdbus_convenience_la_SOURCES= \
UTIL_SOURCES= \
dbus-dataslot.c \
dbus-dataslot.h \
dbus-hash.c \
......@@ -98,7 +89,19 @@ libdbus_convenience_la_SOURCES= \
dbus-sysdeps.c \
dbus-sysdeps.h
libdbus_1_la_LIBADD= $(DBUS_CLIENT_LIBS) libdbus-convenience.la
libdbus_1_la_SOURCES= \
$(DBUS_SOURCES) \
$(UTIL_SOURCES)
libdbus_convenience_la_SOURCES= \
$(DBUS_SOURCES) \
$(UTIL_SOURCES)
## this library is the same as libdbus, but exports all the symbols
## and is only used for static linking within the dbus package.
noinst_LTLIBRARIES=libdbus-convenience.la
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 "^[^_].*"
......@@ -120,7 +123,7 @@ noinst_PROGRAMS=$(TESTS)
dbus_test_SOURCES= \
dbus-test-main.c
dbus_test_LDADD= $(DBUS_CLIENT_LIBS) libdbus-convenience.la libdbus-1.la
dbus_test_LDADD= $(DBUS_CLIENT_LIBS) libdbus-1.la
## mop up the gcov files
clean-local:
......
......@@ -367,6 +367,8 @@ dbus_parse_address (const char *address,
link = _dbus_list_get_next_link (&entries, link);
}
_dbus_list_clear (&entries);
return FALSE;
}
......
......@@ -322,9 +322,9 @@ _dbus_auth_new (int size)
enomem_3:
_dbus_string_free (&auth->identity);
enomem_2:
_dbus_string_free (&auth->incoming);
enomem_1:
_dbus_string_free (&auth->outgoing);
enomem_1:
_dbus_string_free (&auth->incoming);
enomem_0:
dbus_free (auth);
return NULL;
......@@ -1863,7 +1863,9 @@ _dbus_auth_unref (DBusAuth *auth)
if (auth->keyring)
_dbus_keyring_unref (auth->keyring);
_dbus_string_free (&auth->context);
_dbus_string_free (&auth->challenge);
_dbus_string_free (&auth->identity);
_dbus_string_free (&auth->incoming);
_dbus_string_free (&auth->outgoing);
......
......@@ -31,26 +31,177 @@
* @ingroup DBus
* @brief Functions for communicating with the message bus
*
*/
/**
* @defgroup DBusBusInternals Message bus APIs internals
* @ingroup DBusInternals
* @brief Internals of functions for communicating with the message bus
*
* @{
*/
/**
* Block of message-bus-related data we attach to each
* #DBusConnection used with these convenience functions.
*
*/
typedef struct
{
char *base_service;
} BusData;
/** The slot we have reserved to store BusData
*/
static int bus_data_slot = -1;
/** Number of connections using the slot
*/
static int bus_data_slot_refcount = 0;
/**
* Lock for bus_data_slot and bus_data_slot_refcount
*/
static DBusMutex *slot_lock;
/**
* Initialize the mutex used for bus_data_slot
*
* @returns the mutex
*/
DBusMutex *
_dbus_bus_init_lock (void)
{
slot_lock = dbus_mutex_new ();
return slot_lock;
}
static dbus_bool_t
data_slot_ref (void)
{
dbus_mutex_lock (slot_lock);
if (bus_data_slot < 0)
bus_data_slot = dbus_connection_allocate_data_slot ();
if (bus_data_slot < 0)
{
dbus_mutex_unlock (slot_lock);
return FALSE;
}
bus_data_slot_refcount += 1;
dbus_mutex_unlock (slot_lock);
return TRUE;
}
static void
data_slot_unref (void)
{
dbus_mutex_lock (slot_lock);
_dbus_assert (bus_data_slot >= 0);
_dbus_assert (bus_data_slot_refcount > 0);
bus_data_slot_refcount -= 1;
if (bus_data_slot_refcount == 0)
{
dbus_connection_free_data_slot (bus_data_slot);
bus_data_slot = -1;
}
dbus_mutex_unlock (slot_lock);
}
static void
bus_data_free (void *data)
{
BusData *bd = data;
dbus_free (bd->base_service);
dbus_free (bd);
data_slot_unref ();
}
static BusData*
ensure_bus_data (DBusConnection *connection)
{
BusData *bd;
if (!data_slot_ref ())
return NULL;
bd = dbus_connection_get_data (connection, bus_data_slot);
if (bd == NULL)
{
bd = dbus_new0 (BusData, 1);
if (bd == NULL)
{
data_slot_unref ();
return NULL;
}
dbus_connection_set_data (connection, bus_data_slot, bd,
bus_data_free);
/* Data slot refcount now held by the BusData */
}
else
{
data_slot_unref ();
}
return bd;
}
/** @} */ /* end of implementation details docs */
/**
* @addtogroup DBusBus
* @{
*/
/**
* Registers a connection with the bus. This must be the first
* thing an application does when connecting to the message bus.
* If registration succeeds, the base service name will be set,
* and can be obtained using dbus_bus_get_base_service().
*