diff options
| -rw-r--r-- | ChangeLog | 70 | ||||
| -rw-r--r-- | bus/Makefile.am | 15 | ||||
| -rw-r--r-- | bus/bus.c | 39 | ||||
| -rw-r--r-- | bus/connection.c | 140 | ||||
| -rw-r--r-- | bus/dispatch.c | 420 | ||||
| -rw-r--r-- | bus/test.c | 73 | ||||
| -rw-r--r-- | bus/test.h | 9 | ||||
| -rw-r--r-- | dbus/Makefile.am | 29 | ||||
| -rw-r--r-- | dbus/dbus-address.c | 2 | ||||
| -rw-r--r-- | dbus/dbus-auth.c | 8 | ||||
| -rw-r--r-- | dbus/dbus-bus.c | 218 | ||||
| -rw-r--r-- | dbus/dbus-bus.h | 22 | ||||
| -rw-r--r-- | dbus/dbus-connection.c | 73 | ||||
| -rw-r--r-- | dbus/dbus-internals.c | 52 | ||||
| -rw-r--r-- | dbus/dbus-internals.h | 7 | ||||
| -rw-r--r-- | dbus/dbus-list.c | 56 | ||||
| -rw-r--r-- | dbus/dbus-list.h | 1 | ||||
| -rw-r--r-- | dbus/dbus-memory.c | 124 | ||||
| -rw-r--r-- | dbus/dbus-mempool.c | 197 | ||||
| -rw-r--r-- | dbus/dbus-mempool.h | 2 | ||||
| -rw-r--r-- | dbus/dbus-server-debug-pipe.c | 3 | ||||
| -rw-r--r-- | dbus/dbus-string.c | 2 | ||||
| -rw-r--r-- | dbus/dbus-threads.c | 3 | ||||
| -rw-r--r-- | dbus/dbus-transport-unix.c | 6 | ||||
| -rw-r--r-- | test/Makefile.am | 2 |
25 files changed, 1263 insertions, 310 deletions
@@ -1,3 +1,73 @@ +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. diff --git a/bus/Makefile.am b/bus/Makefile.am index 9c2f08bd..aee6e37a 100644 --- a/bus/Makefile.am +++ b/bus/Makefile.am @@ -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); } diff --git a/bus/connection.c b/bus/connection.c index ee3612ae..3308df0f 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -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, diff --git a/bus/dispatch.c b/bus/dispatch.c index d0def78f..0fe4be19 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -2,6 +2,7 @@ /* dispatch.c Message dispatcher * * Copyright (C) 2003 CodeFactory AB + * Copyright (C) 2003 Red Hat, Inc. * * Licensed under the Academic Free License version 1.2 * @@ -33,6 +34,7 @@ #include <string.h> static int message_handler_slot; +static int message_handler_slot_refcount; typedef struct { @@ -329,22 +331,46 @@ bus_dispatch_message_handler (DBusMessageHandler *handler, return DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS; } -dbus_bool_t -bus_dispatch_add_connection (DBusConnection *connection) +static dbus_bool_t +message_handler_slot_ref (void) { - DBusMessageHandler *handler; - message_handler_slot = dbus_connection_allocate_data_slot (); if (message_handler_slot < 0) return FALSE; + message_handler_slot_refcount += 1; + + return TRUE; +} + +static void +message_handler_slot_unref (void) +{ + _dbus_assert (message_handler_slot_refcount > 0); + message_handler_slot_refcount -= 1; + if (message_handler_slot_refcount == 0) + { + dbus_connection_free_data_slot (message_handler_slot); + message_handler_slot = -1; + } +} + +dbus_bool_t +bus_dispatch_add_connection (DBusConnection *connection) +{ + DBusMessageHandler *handler; + + if (!message_handler_slot_ref ()) + return FALSE; + handler = dbus_message_handler_new (bus_dispatch_message_handler, NULL, NULL); if (!dbus_connection_add_filter (connection, handler)) { dbus_message_handler_unref (handler); - + message_handler_slot_unref (); + return FALSE; } @@ -355,6 +381,7 @@ bus_dispatch_add_connection (DBusConnection *connection) { dbus_connection_remove_filter (connection, handler); dbus_message_handler_unref (handler); + message_handler_slot_unref (); return FALSE; } @@ -371,9 +398,9 @@ bus_dispatch_remove_connection (DBusConnection *connection) dbus_connection_set_data (connection, message_handler_slot, NULL, NULL); -} - + message_handler_slot_unref (); +} #ifdef DBUS_BUILD_TESTS @@ -381,6 +408,8 @@ typedef dbus_bool_t (* Check1Func) (BusContext *context); typedef dbus_bool_t (* Check2Func) (BusContext *context, DBusConnection *connection); +static dbus_bool_t check_no_leftovers (BusContext *context); + static void flush_bus (BusContext *context) { @@ -388,13 +417,239 @@ flush_bus (BusContext *context) ; } +typedef struct +{ + const char *expected_service_name; + dbus_bool_t failed; +} CheckServiceDeletedData; + +static dbus_bool_t +check_service_deleted_foreach (DBusConnection *connection, + void *data) +{ + CheckServiceDeletedData *d = data; + DBusMessage *message; + DBusError error; + char *service_name; + + dbus_error_init (&error); + d->failed = TRUE; + service_name = NULL; + + message = dbus_connection_pop_message (connection); + if (message == NULL) + { + _dbus_warn ("Did not receive a message on %p, expecting %s\n", + connection, DBUS_MESSAGE_SERVICE_DELETED); + goto out; + } + else if (!dbus_message_name_is (message, DBUS_MESSAGE_SERVICE_DELETED)) + { + _dbus_warn ("Received message %s on %p, expecting %s\n", + dbus_message_get_name (message), + connection, DBUS_MESSAGE_SERVICE_DELETED); + goto out; + } + else + { + if (!dbus_message_get_args (message, &error, + DBUS_TYPE_STRING, &service_name, + DBUS_TYPE_INVALID)) + { + if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY)) + { + _dbus_verbose ("no memory to get service name arg\n"); + } + else + { + _dbus_assert (dbus_error_is_set (&error)); + _dbus_warn ("Did not get the expected single string argument\n"); + goto out; + } + } + else if (strcmp (service_name, d->expected_service_name) != 0) + { + _dbus_warn ("expected deletion of service %s, got deletion of %s\n", + d->expected_service_name, + service_name); + goto out; + } + } + + d->failed = FALSE; + + out: + dbus_free (service_name); + dbus_error_free (&error); + + if (message) + dbus_message_unref (message); + + return !d->failed; +} + static void -kill_client_connection (DBusConnection *connection) +kill_client_connection (BusContext *context, + DBusConnection *connection) { + char *base_service; + const char *s; + CheckServiceDeletedData csdd; + + _dbus_verbose ("killing connection %p\n", connection); + + s = dbus_bus_get_base_service (connection); + _dbus_assert (s != NULL); + + while ((base_service = _dbus_strdup (s)) == NULL) + bus_wait_for_memory (); + + dbus_connection_ref (connection); + /* kick in the disconnect handler that unrefs the connection */ dbus_connection_disconnect (connection); - while (dbus_connection_dispatch_message (connection)) - ; + + flush_bus (context); + + _dbus_assert (bus_test_client_listed (connection)); + + /* Run disconnect handler in test.c */ + if (dbus_connection_dispatch_message (connection)) + _dbus_assert_not_reached ("something received on connection being killed other than the disconnect"); + + _dbus_assert (!dbus_connection_get_is_connected (connection)); + dbus_connection_unref (connection); + connection = NULL; + _dbus_assert (!bus_test_client_listed (connection)); + + csdd.expected_service_name = base_service; + csdd.failed = FALSE; + + bus_test_clients_foreach (check_service_deleted_foreach, + &csdd); + + dbus_free (base_service); + + if (csdd.failed) + _dbus_assert_not_reached ("didn't get the expected ServiceDeleted messages"); + + if (!check_no_leftovers (context)) + _dbus_assert_not_reached ("stuff left in message queues after disconnecting a client"); +} + +typedef struct +{ + dbus_bool_t failed; +} CheckNoMessagesData; + +static dbus_bool_t +check_no_messages_foreach (DBusConnection *connection, + void *data) +{ + CheckNoMessagesData *d = data; + DBusMessage *message; + + message = dbus_connection_pop_message (connection); + if (message != NULL) + { + _dbus_warn ("Received message %s on %p, expecting no messages\n", + dbus_message_get_name (message), connection); + d->failed = TRUE; + } + + if (message) + dbus_message_unref (message); + return !d->failed; +} + +typedef struct +{ + DBusConnection *skip_connection; + const char *expected_service_name; + dbus_bool_t failed; +} CheckServiceCreatedData; + +static dbus_bool_t +check_service_created_foreach (DBusConnection *connection, + void *data) +{ + CheckServiceCreatedData *d = data; + DBusMessage *message; + DBusError error; + char *service_name; + + if (connection == d->skip_connection) + return TRUE; + + dbus_error_init (&error); + d->failed = TRUE; + service_name = NULL; + + message = dbus_connection_pop_message (connection); + if (message == NULL) + { + _dbus_warn ("Did not receive a message on %p, expecting %s\n", + connection, DBUS_MESSAGE_SERVICE_CREATED); + goto out; + } + else if (!dbus_message_name_is (message, DBUS_MESSAGE_SERVICE_CREATED)) + { + _dbus_warn ("Received message %s on %p, expecting %s\n", + dbus_message_get_name (message), + connection, DBUS_MESSAGE_SERVICE_CREATED); + goto out; + } + else + { + if (!dbus_message_get_args (message, &error, + DBUS_TYPE_STRING, &service_name, + DBUS_TYPE_INVALID)) + { + if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY)) + { + _dbus_verbose ("no memory to get service name arg\n"); + } + else + { + _dbus_assert (dbus_error_is_set (&error)); + _dbus_warn ("Did not get the expected single string argument\n"); + goto out; + } + } + else if (strcmp (service_name, d->expected_service_name) != 0) + { + _dbus_warn ("expected creation of service %s, got creation of %s\n", + d->expected_service_name, + service_name); + goto out; + } + } + + d->failed = FALSE; + + out: + dbus_free (service_name); + dbus_error_free (&error); + + if (message) + dbus_message_unref (message); + + return !d->failed; +} + +static dbus_bool_t +check_no_leftovers (BusContext *context) +{ + CheckNoMessagesData nmd; + + nmd.failed = FALSE; + bus_test_clients_foreach (check_no_messages_foreach, + &nmd); + + if (nmd.failed) + return FALSE; + else + return TRUE; } /* returns TRUE if the correct thing happens, @@ -408,8 +663,12 @@ check_hello_message (BusContext *context, dbus_int32_t serial; dbus_bool_t retval; DBusError error; - + char *name; + char *acquired; + dbus_error_init (&error); + name = NULL; + acquired = NULL; message = dbus_message_new (DBUS_SERVICE_DBUS, DBUS_MESSAGE_HELLO); @@ -460,7 +719,7 @@ check_hello_message (BusContext *context, } else { - char *str; + CheckServiceCreatedData scd; if (dbus_message_name_is (message, DBUS_MESSAGE_HELLO)) @@ -474,21 +733,91 @@ check_hello_message (BusContext *context, goto out; } + retry_get_hello_name: if (!dbus_message_get_args (message, &error, - DBUS_TYPE_STRING, &str, + DBUS_TYPE_STRING, &name, DBUS_TYPE_INVALID)) { - _dbus_warn ("Did not get the expected single string argument\n"); + if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY)) + { + _dbus_verbose ("no memory to get service name arg from hello\n"); + dbus_error_free (&error); + bus_wait_for_memory (); + goto retry_get_hello_name; + } + else + { + _dbus_assert (dbus_error_is_set (&error)); + _dbus_warn ("Did not get the expected single string argument to hello\n"); + goto out; + } + } + + _dbus_verbose ("Got hello name: %s\n", name); + + while (!dbus_bus_set_base_service (connection, name)) + bus_wait_for_memory (); + + scd.skip_connection = NULL; + scd.failed = FALSE; + scd.expected_service_name = name; + bus_test_clients_foreach (check_service_created_foreach, + &scd); + + if (scd.failed) + goto out; + + /* Client should also have gotten ServiceAcquired */ + dbus_message_unref (message); + message = dbus_connection_pop_message (connection); + if (message == NULL) + { + _dbus_warn ("Expecting %s, got nothing\n", + DBUS_MESSAGE_SERVICE_ACQUIRED); goto out; } + + retry_get_acquired_name: + if (!dbus_message_get_args (message, &error, + DBUS_TYPE_STRING, &acquired, + DBUS_TYPE_INVALID)) + { + if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY)) + { + _dbus_verbose ("no memory to get service name arg from acquired\n"); + dbus_error_free (&error); + bus_wait_for_memory (); + goto retry_get_acquired_name; + } + else + { + _dbus_assert (dbus_error_is_set (&error)); + _dbus_warn ("Did not get the expected single string argument to ServiceAcquired\n"); + goto out; + } + } + + _dbus_verbose ("Got acquired name: %s\n", acquired); - _dbus_verbose ("Got hello name: %s\n", str); - dbus_free (str); + if (strcmp (acquired, name) != 0) + { + _dbus_warn ("Acquired name is %s but expected %s\n", + acquired, name); + goto out; + } } - + + if (!check_no_leftovers (context)) + goto out; + retval = TRUE; out: + dbus_error_free (&error); + + dbus_free (name); + dbus_free (acquired); + if (message) dbus_message_unref (message); @@ -522,7 +851,24 @@ check_hello_connection (BusContext *context) if (!check_hello_message (context, connection)) return FALSE; - kill_client_connection (connection); + if (dbus_bus_get_base_service (connection) == NULL) + { + /* We didn't successfully register, so we can't + * do the usual kill_client_connection() checks + */ + dbus_connection_ref (connection); + dbus_connection_disconnect (connection); + /* dispatching disconnect handler will unref once */ + if (dbus_connection_dispatch_message (connection)) + _dbus_assert_not_reached ("message other than disconnect dispatched after failure to register"); + dbus_connection_unref (connection); + _dbus_assert (!bus_test_client_listed (connection)); + return TRUE; + } + else + { + kill_client_connection (context, connection); + } return TRUE; } @@ -560,6 +906,9 @@ check1_try_iterations (BusContext *context, if (! (*func) (context)) _dbus_assert_not_reached ("test failed"); + if (!check_no_leftovers (context)) + _dbus_assert_not_reached ("Messages were left over, should be covered by test suite"); + approx_mallocs -= 1; } @@ -588,23 +937,29 @@ bus_dispatch_test (const DBusString *test_data_dir) if (foo == NULL) _dbus_assert_not_reached ("could not alloc connection"); + if (!bus_setup_debug_client (foo)) + _dbus_assert_not_reached ("could not set up connection"); + + if (!check_hello_message (context, foo)) + _dbus_assert_not_reached ("hello message failed"); + bar = dbus_connection_open ("debug-pipe:name=test-server", &result); if (bar == NULL) _dbus_assert_not_reached ("could not alloc connection"); + if (!bus_setup_debug_client (bar)) + _dbus_assert_not_reached ("could not set up connection"); + + if (!check_hello_message (context, bar)) + _dbus_assert_not_reached ("hello message failed"); + baz = dbus_connection_open ("debug-pipe:name=test-server", &result); if (baz == NULL) _dbus_assert_not_reached ("could not alloc connection"); - if (!bus_setup_debug_client (foo) || - !bus_setup_debug_client (bar) || - !bus_setup_debug_client (baz)) + if (!bus_setup_debug_client (baz)) _dbus_assert_not_reached ("could not set up connection"); - - if (!check_hello_message (context, foo)) - _dbus_assert_not_reached ("hello message failed"); - if (!check_hello_message (context, bar)) - _dbus_assert_not_reached ("hello message failed"); + if (!check_hello_message (context, baz)) _dbus_assert_not_reached ("hello message failed"); @@ -612,11 +967,24 @@ bus_dispatch_test (const DBusString *test_data_dir) check_hello_connection); dbus_connection_disconnect (foo); + if (dbus_connection_dispatch_message (foo)) + _dbus_assert_not_reached ("extra message in queue"); dbus_connection_unref (foo); + _dbus_assert (!bus_test_client_listed (foo)); + dbus_connection_disconnect (bar); + if (dbus_connection_dispatch_message (bar)) + _dbus_assert_not_reached ("extra message in queue"); dbus_connection_unref (bar); + _dbus_assert (!bus_test_client_listed (bar)); + dbus_connection_disconnect (baz); + if (dbus_connection_dispatch_message (baz)) + _dbus_assert_not_reached ("extra message in queue"); dbus_connection_unref (baz); + _dbus_assert (!bus_test_client_listed (baz)); + + bus_context_unref (context); return TRUE; } @@ -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 diff --git a/dbus/Makefile.am b/dbus/Makefile.am index c66c5367..20be7919 100644 --- a/dbus/Makefile.am +++ b/dbus/Makefile.am @@ -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: diff --git a/dbus/dbus-address.c b/dbus/dbus-address.c index 25179cea..87a99bd8 100644 --- a/dbus/dbus-address.c +++ b/dbus/dbus-address.c @@ -367,6 +367,8 @@ dbus_parse_address (const char *address, link = _dbus_list_get_next_link (&entries, link); } + _dbus_list_clear (&entries); + return FALSE; } diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c index 73454bc1..4b1f5504 100644 --- a/dbus/dbus-auth.c +++ b/dbus/dbus-auth.c @@ -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); diff --git a/dbus/dbus-bus.c b/dbus/dbus-bus.c index cc612a78..3e409257 100644 --- a/dbus/dbus-bus.c +++ b/dbus/dbus-bus.c @@ -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(). * * @todo if we get an error reply, it has to be converted into * DBusError and returned * * @param connection the connection * @param error place to store errors - * @returns the client's unique service name, #NULL on error + * @returns #TRUE on success */ -char* -dbus_bus_register_client (DBusConnection *connection, - DBusError *error) +dbus_bool_t +dbus_bus_register (DBusConnection *connection, + DBusError *error) { DBusMessage *message, *reply; char *name; + BusData *bd; + + bd = ensure_bus_data (connection); + if (bd == NULL) + { + _DBUS_SET_OOM (error); + return FALSE; + } + + if (bd->base_service != NULL) + { + _dbus_warn ("Attempt to register the same DBusConnection with the message bus, but it is already registered\n"); + /* This isn't an error, it's a programming bug. We'll be nice + * and not _dbus_assert_not_reached() + */ + return TRUE; + } message = dbus_message_new (DBUS_SERVICE_DBUS, DBUS_MESSAGE_HELLO); @@ -58,7 +209,7 @@ dbus_bus_register_client (DBusConnection *connection, if (!message) { _DBUS_SET_OOM (error); - return NULL; + return FALSE; } reply = dbus_connection_send_with_reply_and_block (connection, message, -1, error); @@ -68,7 +219,7 @@ dbus_bus_register_client (DBusConnection *connection, if (reply == NULL) { _DBUS_ASSERT_ERROR_IS_SET (error); - return NULL; + return FALSE; } if (!dbus_message_get_args (reply, error, @@ -76,10 +227,61 @@ dbus_bus_register_client (DBusConnection *connection, 0)) { _DBUS_ASSERT_ERROR_IS_SET (error); - return NULL; + return FALSE; } + + bd->base_service = name; + + return TRUE; +} + + +/** + * Sets the base service name of the connection. + * Can only be used if you registered with the + * bus manually (i.e. if you did not call + * dbus_bus_register()). Can only be called + * once per connection. + * + * @param connection the connection + * @param the base service name + * @returns #FALSE if not enough memory + */ +dbus_bool_t +dbus_bus_set_base_service (DBusConnection *connection, + const char *base_service) +{ + BusData *bd; + + bd = ensure_bus_data (connection); + if (bd == NULL) + return FALSE; + + _dbus_assert (bd->base_service == NULL); + _dbus_assert (base_service != NULL); + + bd->base_service = _dbus_strdup (base_service); + return bd->base_service != NULL; +} + +/** + * Gets the base service name of the connection. + * Only possible after the connection has been registered + * with the message bus. + * + * @param connection the connection + * @returns the base service name + */ +const char* +dbus_bus_get_base_service (DBusConnection *connection) +{ + BusData *bd; + + bd = ensure_bus_data (connection); + if (bd == NULL) + return NULL; - return name; + return bd->base_service; } /** diff --git a/dbus/dbus-bus.h b/dbus/dbus-bus.h index d1c2bfd7..c6800ede 100644 --- a/dbus/dbus-bus.h +++ b/dbus/dbus-bus.h @@ -29,15 +29,19 @@ #include <dbus/dbus-connection.h> -char* dbus_bus_register_client (DBusConnection *connection, - DBusError *error); -int dbus_bus_acquire_service (DBusConnection *connection, - const char *service_name, - unsigned int flags, - DBusError *error); -dbus_bool_t dbus_bus_service_exists (DBusConnection *connection, - const char *service_name, - DBusError *error); +dbus_bool_t dbus_bus_register (DBusConnection *connection, + DBusError *error); +dbus_bool_t dbus_bus_set_base_service (DBusConnection *connection, + const char *base_service); +const char* dbus_bus_get_base_service (DBusConnection *connection); +int dbus_bus_acquire_service (DBusConnection *connection, + const char *service_name, + unsigned int flags, + DBusError *error); +dbus_bool_t dbus_bus_service_exists (DBusConnection *connection, + const char *service_name, + DBusError *error); + #endif /* DBUS_BUS_H */ diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 780c410f..6f02d258 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -924,6 +924,10 @@ _dbus_connection_last_unref (DBusConnection *connection) * it if the count reaches zero. It is a bug to drop the last reference * to a connection that has not been disconnected. * + * @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. + * * @param connection the connection. */ void @@ -1109,10 +1113,10 @@ dbus_connection_send_preallocated (DBusConnection *connection, * fail is lack of memory. Even if the connection is disconnected, * no error will be returned. * - * If the function fails, it returns #FALSE and returns the - * reason for failure via the result parameter. - * The result parameter can be #NULL if you aren't interested - * in the reason for the failure. + * If the function fails due to lack of memory, it returns #FALSE. + * The function will never fail for other reasons; even if the + * connection is disconnected, you can queue an outgoing message, + * though obviously it won't be sent. * * @param connection the connection. * @param message the message to write. @@ -1582,26 +1586,49 @@ dbus_connection_steal_borrowed_message (DBusConnection *connection, dbus_mutex_unlock (connection->mutex); } - /* See dbus_connection_pop_message, but requires the caller to own * the lock before calling. May drop the lock while running. */ -static DBusMessage* -_dbus_connection_pop_message_unlocked (DBusConnection *connection) +static DBusList* +_dbus_connection_pop_message_link_unlocked (DBusConnection *connection) { if (connection->message_borrowed != NULL) _dbus_connection_wait_for_borrowed (connection); if (connection->n_incoming > 0) { - DBusMessage *message; + DBusList *link; - message = _dbus_list_pop_first (&connection->incoming_messages); + link = _dbus_list_pop_first_link (&connection->incoming_messages); connection->n_incoming -= 1; _dbus_verbose ("Message %p removed from incoming queue %p, %d incoming\n", - message, connection, connection->n_incoming); + link->data, connection, connection->n_incoming); + + return link; + } + else + return NULL; +} + +/* See dbus_connection_pop_message, but requires the caller to own + * the lock before calling. May drop the lock while running. + */ +static DBusMessage* +_dbus_connection_pop_message_unlocked (DBusConnection *connection) +{ + DBusList *link; + + link = _dbus_connection_pop_message_link_unlocked (connection); + if (link != NULL) + { + DBusMessage *message; + + message = link->data; + + _dbus_list_free_link (link); + return message; } else @@ -1690,16 +1717,12 @@ dbus_connection_dispatch_message (DBusConnection *connection) ReplyHandlerData *reply_handler_data; const char *name; dbus_int32_t reply_serial; - - /* Preallocate link so we can put the message back on failure */ - message_link = _dbus_list_alloc_link (NULL); - if (message_link == NULL) - return FALSE; dbus_mutex_lock (connection->mutex); /* We need to ref the connection since the callback could potentially - * drop the last ref to it */ + * drop the last ref to it + */ _dbus_connection_ref_unlocked (connection); _dbus_connection_acquire_dispatch (connection); @@ -1710,16 +1733,16 @@ dbus_connection_dispatch_message (DBusConnection *connection) * protected by the lock, since only one will get the lock, and that * one will finish the message dispatching */ - message = _dbus_connection_pop_message_unlocked (connection); - if (message == NULL) + message_link = _dbus_connection_pop_message_link_unlocked (connection); + if (message_link == NULL) { _dbus_connection_release_dispatch (connection); dbus_mutex_unlock (connection->mutex); dbus_connection_unref (connection); return FALSE; } - - message_link->data = message; + + message = message_link->data; result = DBUS_HANDLER_RESULT_ALLOW_MORE_HANDLERS; @@ -1751,6 +1774,7 @@ dbus_connection_dispatch_message (DBusConnection *connection) DBusMessageHandler *handler = link->data; DBusList *next = _dbus_list_get_next_link (&filter_list_copy, link); + _dbus_verbose (" running filter on message %p\n", message); result = _dbus_message_handler_handle_message (handler, connection, message); @@ -1790,6 +1814,9 @@ dbus_connection_dispatch_message (DBusConnection *connection) if (reply_handler_data) { dbus_mutex_unlock (connection->mutex); + + _dbus_verbose (" running reply handler on message %p\n", message); + result = _dbus_message_handler_handle_message (reply_handler_data->handler, connection, message); reply_handler_data_free (reply_handler_data); @@ -1807,6 +1834,9 @@ dbus_connection_dispatch_message (DBusConnection *connection) /* We're still protected from dispatch_message reentrancy here * since we acquired the dispatcher */ dbus_mutex_unlock (connection->mutex); + + _dbus_verbose (" running app handler on message %p\n", message); + result = _dbus_message_handler_handle_message (handler, connection, message); dbus_mutex_lock (connection->mutex); @@ -1815,6 +1845,9 @@ dbus_connection_dispatch_message (DBusConnection *connection) } } + _dbus_verbose (" done dispatching %p (%s)\n", message, + dbus_message_get_name (message)); + out: _dbus_connection_release_dispatch (connection); dbus_mutex_unlock (connection->mutex); diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c index acd6d72f..5153a767 100644 --- a/dbus/dbus-internals.c +++ b/dbus/dbus-internals.c @@ -291,56 +291,4 @@ _dbus_type_to_string (int type) } } -#ifdef DBUS_BUILD_TESTS -static int fail_alloc_counter = _DBUS_INT_MAX; -/** - * Sets the number of allocations until we simulate a failed - * allocation. If set to 0, the next allocation to run - * fails; if set to 1, one succeeds then the next fails; etc. - * Set to _DBUS_INT_MAX to not fail anything. - * - * @param until_next_fail number of successful allocs before one fails - */ -void -_dbus_set_fail_alloc_counter (int until_next_fail) -{ - fail_alloc_counter = until_next_fail; -} - -/** - * Gets the number of successful allocs until we'll simulate - * a failed alloc. - * - * @returns current counter value - */ -int -_dbus_get_fail_alloc_counter (void) -{ - return fail_alloc_counter; -} - -/** - * Called when about to alloc some memory; if - * it returns #TRUE, then the allocation should - * fail. If it returns #FALSE, then the allocation - * should not fail. - * - * @returns #TRUE if this alloc should fail - */ -dbus_bool_t -_dbus_decrement_fail_alloc_counter (void) -{ - if (fail_alloc_counter <= 0) - { - fail_alloc_counter = _DBUS_INT_MAX; - return TRUE; - } - else - { - fail_alloc_counter -= 1; - return FALSE; - } -} -#endif /* DBUS_BUILD_TESTS */ - /** @} */ diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index 2576982d..559b38ec 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -156,10 +156,16 @@ extern const char _dbus_no_memory_message[]; void _dbus_set_fail_alloc_counter (int until_next_fail); int _dbus_get_fail_alloc_counter (void); dbus_bool_t _dbus_decrement_fail_alloc_counter (void); +dbus_bool_t _dbus_disable_mem_pools (void); #else #define _dbus_set_fail_alloc_counter(n) #define _dbus_get_fail_alloc_counter _DBUS_INT_MAX + +/* These are constant expressions so that blocks + * they protect should be optimized away + */ #define _dbus_decrement_fail_alloc_counter() FALSE +#define _dbus_disable_mem_pools() FALSE #endif /* !DBUS_BUILD_TESTS */ /* Thread initializers */ @@ -169,6 +175,7 @@ DBusMutex *_dbus_server_slots_init_lock (void); DBusMutex *_dbus_atomic_init_lock (void); DBusMutex *_dbus_message_handler_init_lock (void); DBusMutex *_dbus_user_info_init_lock (void); +DBusMutex *_dbus_bus_init_lock (void); DBUS_END_DECLS; diff --git a/dbus/dbus-list.c b/dbus/dbus-list.c index d0ca8dfa..4c530dc7 100644 --- a/dbus/dbus-list.c +++ b/dbus/dbus-list.c @@ -94,7 +94,11 @@ static void free_link (DBusList *link) { dbus_mutex_lock (list_pool_lock); - _dbus_mem_pool_dealloc (list_pool, link); + if (_dbus_mem_pool_dealloc (list_pool, link)) + { + _dbus_mem_pool_free (list_pool); + list_pool = NULL; + } dbus_mutex_unlock (list_pool_lock); } @@ -429,21 +433,14 @@ _dbus_list_remove_last (DBusList **list, return FALSE; } -/** - * Removes a link from the list. This is a constant-time operation. - * - * @param list address of the list head. - * @param link the list link to remove. - */ -void -_dbus_list_remove_link (DBusList **list, - DBusList *link) +static void +_dbus_list_unlink (DBusList **list, + DBusList *link) { if (link->next == link) { /* one-element list */ *list = NULL; - free_link (link); } else { @@ -452,12 +449,24 @@ _dbus_list_remove_link (DBusList **list, if (*list == link) *list = link->next; - - free_link (link); } } /** + * Removes a link from the list. This is a constant-time operation. + * + * @param list address of the list head. + * @param link the list link to remove. + */ +void +_dbus_list_remove_link (DBusList **list, + DBusList *link) +{ + _dbus_list_unlink (list, link); + free_link (link); +} + +/** * Frees all links in the list and sets the list head to #NULL. Does * not free the data in each link, for obvious reasons. This is a * linear-time operation. @@ -544,6 +553,27 @@ _dbus_list_get_first (DBusList **list) } /** + * Removes the first link in the list and returns it. This is a + * constant-time operation. + * + * @param list address of the list head. + * @returns the first link in the list, or #NULL for an empty list. + */ +DBusList* +_dbus_list_pop_first_link (DBusList **list) +{ + DBusList *link; + + link = _dbus_list_get_first_link (list); + if (link == NULL) + return NULL; + + _dbus_list_unlink (list, link); + + return link; +} + +/** * Removes the first value in the list and returns it. This is a * constant-time operation. * diff --git a/dbus/dbus-list.h b/dbus/dbus-list.h index 3f23f2ec..df068568 100644 --- a/dbus/dbus-list.h +++ b/dbus/dbus-list.h @@ -62,6 +62,7 @@ void* _dbus_list_get_last (DBusList **list); void* _dbus_list_get_first (DBusList **list); void* _dbus_list_pop_first (DBusList **list); void* _dbus_list_pop_last (DBusList **list); +DBusList* _dbus_list_pop_first_link (DBusList **list); dbus_bool_t _dbus_list_copy (DBusList **list, DBusList **dest); int _dbus_list_get_length (DBusList **list); diff --git a/dbus/dbus-memory.c b/dbus/dbus-memory.c index e1075228..a426b371 100644 --- a/dbus/dbus-memory.c +++ b/dbus/dbus-memory.c @@ -23,6 +23,7 @@ #include "dbus-memory.h" #include "dbus-internals.h" +#include "dbus-sysdeps.h" #include <stdlib.h> @@ -73,10 +74,13 @@ */ #ifdef DBUS_BUILD_TESTS -static dbus_bool_t inited = FALSE; +static dbus_bool_t debug_initialized = FALSE; static int fail_counts = -1; static size_t fail_size = 0; +static int fail_alloc_counter = _DBUS_INT_MAX; static dbus_bool_t guards = FALSE; +static dbus_bool_t disable_mem_pools = FALSE; + /** value stored in guard padding for debugging buffer overrun */ #define GUARD_VALUE 0xdeadbeef /** size of the information about the block stored in guard mode */ @@ -89,27 +93,114 @@ static dbus_bool_t guards = FALSE; #define GUARD_START_OFFSET (GUARD_START_PAD + GUARD_INFO_SIZE) /** total extra size over the requested allocation for guard stuff */ #define GUARD_EXTRA_SIZE (GUARD_START_OFFSET + GUARD_END_PAD) -#endif -#ifdef DBUS_BUILD_TESTS static void -initialize_malloc_debug (void) +_dbus_initialize_malloc_debug (void) { - if (!inited) + if (!debug_initialized) { + debug_initialized = TRUE; + if (_dbus_getenv ("DBUS_MALLOC_FAIL_NTH") != NULL) { fail_counts = atoi (_dbus_getenv ("DBUS_MALLOC_FAIL_NTH")); - _dbus_set_fail_alloc_counter (fail_counts); + fail_alloc_counter = fail_counts; + _dbus_verbose ("Will fail malloc every %d times\n", fail_counts); } if (_dbus_getenv ("DBUS_MALLOC_FAIL_GREATER_THAN") != NULL) - fail_size = atoi (_dbus_getenv ("DBUS_MALLOC_FAIL_GREATER_THAN")); + { + fail_size = atoi (_dbus_getenv ("DBUS_MALLOC_FAIL_GREATER_THAN")); + _dbus_verbose ("Will fail mallocs over %d bytes\n", + fail_size); + } if (_dbus_getenv ("DBUS_MALLOC_GUARDS") != NULL) - guards = TRUE; + { + guards = TRUE; + _dbus_verbose ("Will use malloc guards\n"); + } + + if (_dbus_getenv ("DBUS_DISABLE_MEM_POOLS") != NULL) + { + disable_mem_pools = TRUE; + _dbus_verbose ("Will disable memory pools\n"); + } + } +} + +/** + * Whether to turn off mem pools, useful for leak checking. + * + * @returns #TRUE if mempools should not be used. + */ +dbus_bool_t +_dbus_disable_mem_pools (void) +{ + _dbus_initialize_malloc_debug (); + return disable_mem_pools; +} + +/** + * Sets the number of allocations until we simulate a failed + * allocation. If set to 0, the next allocation to run + * fails; if set to 1, one succeeds then the next fails; etc. + * Set to _DBUS_INT_MAX to not fail anything. + * + * @param until_next_fail number of successful allocs before one fails + */ +void +_dbus_set_fail_alloc_counter (int until_next_fail) +{ + _dbus_initialize_malloc_debug (); + + fail_alloc_counter = until_next_fail; + + _dbus_verbose ("Set fail alloc counter = %d\n", fail_alloc_counter); +} + +/** + * Gets the number of successful allocs until we'll simulate + * a failed alloc. + * + * @returns current counter value + */ +int +_dbus_get_fail_alloc_counter (void) +{ + _dbus_initialize_malloc_debug (); + + return fail_alloc_counter; +} + +/** + * Called when about to alloc some memory; if + * it returns #TRUE, then the allocation should + * fail. If it returns #FALSE, then the allocation + * should not fail. + * + * @returns #TRUE if this alloc should fail + */ +dbus_bool_t +_dbus_decrement_fail_alloc_counter (void) +{ + _dbus_initialize_malloc_debug (); + + if (fail_alloc_counter <= 0) + { + if (fail_counts >= 0) + fail_alloc_counter = fail_counts; + else + fail_alloc_counter = _DBUS_INT_MAX; + + _dbus_verbose ("reset fail alloc counter to %d\n", fail_alloc_counter); - inited = TRUE; + return TRUE; + } + else + { + fail_alloc_counter -= 1; + return FALSE; } } @@ -250,13 +341,10 @@ void* dbus_malloc (size_t bytes) { #ifdef DBUS_BUILD_TESTS - initialize_malloc_debug (); + _dbus_initialize_malloc_debug (); if (_dbus_decrement_fail_alloc_counter ()) { - if (fail_counts != -1) - _dbus_set_fail_alloc_counter (fail_counts); - _dbus_verbose (" FAILING malloc of %d bytes\n", bytes); return NULL; @@ -293,13 +381,10 @@ void* dbus_malloc0 (size_t bytes) { #ifdef DBUS_BUILD_TESTS - initialize_malloc_debug (); + _dbus_initialize_malloc_debug (); if (_dbus_decrement_fail_alloc_counter ()) { - if (fail_counts != -1) - _dbus_set_fail_alloc_counter (fail_counts); - _dbus_verbose (" FAILING malloc0 of %d bytes\n", bytes); return NULL; @@ -338,13 +423,10 @@ dbus_realloc (void *memory, size_t bytes) { #ifdef DBUS_BUILD_TESTS - initialize_malloc_debug (); + _dbus_initialize_malloc_debug (); if (_dbus_decrement_fail_alloc_counter ()) { - if (fail_counts != -1) - _dbus_set_fail_alloc_counter (fail_counts); - _dbus_verbose (" FAILING realloc of %d bytes\n", bytes); return NULL; diff --git a/dbus/dbus-mempool.c b/dbus/dbus-mempool.c index 9a0f6188..437dbfdc 100644 --- a/dbus/dbus-mempool.c +++ b/dbus/dbus-mempool.c @@ -22,6 +22,7 @@ */ #include "dbus-mempool.h" +#include "dbus-internals.h" /** * @defgroup DBusMemPool memory pools @@ -99,6 +100,7 @@ struct DBusMemPool DBusFreedElement *free_elements; /**< a free list of elements to recycle */ DBusMemBlock *blocks; /**< blocks of memory from malloc() */ + int allocated_elements; /* Count of outstanding allocated elements */ }; /** @} */ @@ -156,6 +158,8 @@ _dbus_mem_pool_new (int element_size, pool->zero_elements = zero_elements != FALSE; + pool->allocated_elements = 0; + /* pick a size for the first block; it increases * for each block we need to allocate. This is * actually half the initial block size @@ -202,80 +206,118 @@ _dbus_mem_pool_free (DBusMemPool *pool) void* _dbus_mem_pool_alloc (DBusMemPool *pool) { - if (_dbus_decrement_fail_alloc_counter ()) - { - _dbus_verbose (" FAILING mempool alloc\n"); - return NULL; - } - - if (pool->free_elements) + if (_dbus_disable_mem_pools ()) { - DBusFreedElement *element = pool->free_elements; + DBusMemBlock *block; + int alloc_size; + + /* This is obviously really silly, but it's + * debug-mode-only code that is compiled out + * when tests are disabled (_dbus_disable_mem_pools() + * is a constant expression FALSE so this block + * should vanish) + */ + + alloc_size = sizeof (DBusMemBlock) - ELEMENT_PADDING + + pool->element_size; + + if (pool->zero_elements) + block = dbus_malloc0 (alloc_size); + else + block = dbus_malloc (alloc_size); - pool->free_elements = pool->free_elements->next; + if (block != NULL) + { + block->next = pool->blocks; + pool->blocks = block; + pool->allocated_elements += 1; - if (pool->zero_elements) - memset (element, '\0', pool->element_size); - - return element; + return (void*) &block->elements[0]; + } + else + return NULL; } else { - void *element; + if (_dbus_decrement_fail_alloc_counter ()) + { + _dbus_verbose (" FAILING mempool alloc\n"); + return NULL; + } + else if (pool->free_elements) + { + DBusFreedElement *element = pool->free_elements; + + pool->free_elements = pool->free_elements->next; + + if (pool->zero_elements) + memset (element, '\0', pool->element_size); + + pool->allocated_elements += 1; - if (pool->blocks == NULL || - pool->blocks->used_so_far == pool->block_size) + return element; + } + else { - /* Need a new block */ - DBusMemBlock *block; - int alloc_size; + void *element; + + if (pool->blocks == NULL || + pool->blocks->used_so_far == pool->block_size) + { + /* Need a new block */ + DBusMemBlock *block; + int alloc_size; #ifdef DBUS_BUILD_TESTS - int saved_counter; + int saved_counter; #endif - if (pool->block_size <= _DBUS_INT_MAX / 4) /* avoid overflow */ - { - /* use a larger block size for our next block */ - pool->block_size *= 2; - _dbus_assert ((pool->block_size % - pool->element_size) == 0); - } + if (pool->block_size <= _DBUS_INT_MAX / 4) /* avoid overflow */ + { + /* use a larger block size for our next block */ + pool->block_size *= 2; + _dbus_assert ((pool->block_size % + pool->element_size) == 0); + } - alloc_size = sizeof (DBusMemBlock) - ELEMENT_PADDING + pool->block_size; + alloc_size = sizeof (DBusMemBlock) - ELEMENT_PADDING + pool->block_size; #ifdef DBUS_BUILD_TESTS - /* We save/restore the counter, so that memory pools won't - * cause a given function to have different number of - * allocations on different invocations. i.e. when testing - * we want consistent alloc patterns. So we skip our - * malloc here for purposes of failed alloc simulation. - */ - saved_counter = _dbus_get_fail_alloc_counter (); - _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); + /* We save/restore the counter, so that memory pools won't + * cause a given function to have different number of + * allocations on different invocations. i.e. when testing + * we want consistent alloc patterns. So we skip our + * malloc here for purposes of failed alloc simulation. + */ + saved_counter = _dbus_get_fail_alloc_counter (); + _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); #endif - if (pool->zero_elements) - block = dbus_malloc0 (alloc_size); - else - block = dbus_malloc (alloc_size); + if (pool->zero_elements) + block = dbus_malloc0 (alloc_size); + else + block = dbus_malloc (alloc_size); #ifdef DBUS_BUILD_TESTS - _dbus_set_fail_alloc_counter (saved_counter); + _dbus_set_fail_alloc_counter (saved_counter); + _dbus_assert (saved_counter == _dbus_get_fail_alloc_counter ()); #endif - if (block == NULL) - return NULL; + if (block == NULL) + return NULL; - block->used_so_far = 0; - block->next = pool->blocks; - pool->blocks = block; - } + block->used_so_far = 0; + block->next = pool->blocks; + pool->blocks = block; + } - element = &pool->blocks->elements[pool->blocks->used_so_far]; + element = &pool->blocks->elements[pool->blocks->used_so_far]; - pool->blocks->used_so_far += pool->element_size; + pool->blocks->used_so_far += pool->element_size; - return element; + pool->allocated_elements += 1; + + return element; + } } } @@ -285,16 +327,61 @@ _dbus_mem_pool_alloc (DBusMemPool *pool) * must have come from this same pool. * @param pool the memory pool * @param element the element earlier allocated. + * @returns #TRUE if there are no remaining allocated elements */ -void +dbus_bool_t _dbus_mem_pool_dealloc (DBusMemPool *pool, void *element) { - DBusFreedElement *freed; + if (_dbus_disable_mem_pools ()) + { + DBusMemBlock *block; + DBusMemBlock *prev; + + /* mmm, fast. ;-) debug-only code, so doesn't matter. */ + + prev = NULL; + block = pool->blocks; - freed = element; - freed->next = pool->free_elements; - pool->free_elements = freed; + while (block != NULL) + { + if (block->elements == (unsigned char*) element) + { + if (prev) + prev->next = block->next; + else + pool->blocks = block->next; + + dbus_free (block); + + _dbus_assert (pool->allocated_elements > 0); + pool->allocated_elements -= 1; + + if (pool->allocated_elements == 0) + _dbus_assert (pool->blocks == NULL); + + return pool->blocks == NULL; + } + prev = block; + block = block->next; + } + + _dbus_assert_not_reached ("freed nonexistent block"); + return FALSE; + } + else + { + DBusFreedElement *freed; + + freed = element; + freed->next = pool->free_elements; + pool->free_elements = freed; + + _dbus_assert (pool->allocated_elements > 0); + pool->allocated_elements -= 1; + + return pool->allocated_elements == 0; + } } /** @} */ diff --git a/dbus/dbus-mempool.h b/dbus/dbus-mempool.h index 17cfa309..3c123915 100644 --- a/dbus/dbus-mempool.h +++ b/dbus/dbus-mempool.h @@ -36,7 +36,7 @@ DBusMemPool* _dbus_mem_pool_new (int element_size, dbus_bool_t zero_elements); void _dbus_mem_pool_free (DBusMemPool *pool); void* _dbus_mem_pool_alloc (DBusMemPool *pool); -void _dbus_mem_pool_dealloc (DBusMemPool *pool, +dbus_bool_t _dbus_mem_pool_dealloc (DBusMemPool *pool, void *element); DBUS_END_DECLS; diff --git a/dbus/dbus-server-debug-pipe.c b/dbus/dbus-server-debug-pipe.c index ceba7659..8f57ae8d 100644 --- a/dbus/dbus-server-debug-pipe.c +++ b/dbus/dbus-server-debug-pipe.c @@ -65,8 +65,11 @@ static DBusHashTable *server_pipe_hash; static void debug_finalize (DBusServer *server) { + DBusServerDebugPipe *debug_server = (DBusServerDebugPipe*) server; + _dbus_server_finalize_base (server); + dbus_free (debug_server->name); dbus_free (server); } diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index f19a30db..cc66b38b 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -162,8 +162,8 @@ undo_alignment (DBusRealString *real) real->str, real->len + 1); - real->align_offset = 0; real->str = real->str - real->align_offset; + real->align_offset = 0; } } diff --git a/dbus/dbus-threads.c b/dbus/dbus-threads.c index 08cebf50..ec83180d 100644 --- a/dbus/dbus-threads.c +++ b/dbus/dbus-threads.c @@ -209,7 +209,8 @@ init_static_locks(void) {&_dbus_connection_slots_init_lock}, {&_dbus_atomic_init_lock}, {&_dbus_message_handler_init_lock}, - {&_dbus_user_info_init_lock} + {&_dbus_user_info_init_lock}, + {&_dbus_bus_init_lock} }; for (i = 0; i < _DBUS_N_ELEMENTS (static_locks); i++) diff --git a/dbus/dbus-transport-unix.c b/dbus/dbus-transport-unix.c index 23ab7c54..17d74886 100644 --- a/dbus/dbus-transport-unix.c +++ b/dbus/dbus-transport-unix.c @@ -189,8 +189,10 @@ queue_messages (DBusTransport *transport) _dbus_verbose ("queueing received message %p\n", message); _dbus_message_add_size_counter (message, transport->live_messages_size); - _dbus_connection_queue_received_message (transport->connection, - message); + if (!_dbus_connection_queue_received_message (transport->connection, + message)) + /* FIXME woops! */; + dbus_message_unref (message); } diff --git a/test/Makefile.am b/test/Makefile.am index 8c527bef..f900b021 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -35,7 +35,7 @@ break_loader_SOURCES= \ spawn_test_SOURCES= \ spawn-test.c -TEST_LIBS=$(DBUS_TEST_LIBS) $(top_builddir)/dbus/libdbus-convenience.la $(top_builddir)/dbus/libdbus-1.la +TEST_LIBS=$(DBUS_TEST_LIBS) $(top_builddir)/dbus/libdbus-convenience.la echo_client_LDADD=$(TEST_LIBS) echo_server_LDADD=$(TEST_LIBS) |