From: Havoc Pennington Date: Mon, 17 Mar 2003 05:39:10 +0000 (+0000) Subject: 2003-03-17 Havoc Pennington X-Git-Tag: dbus-0.6~4 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f7c24715b5489b28b47499eb252b941b735fa1bc;p=platform%2Fupstream%2Fdbus.git 2003-03-17 Havoc Pennington All tests pass, no memleaks, no valgrind complaints. * bus/test.c: refcount handler_slot * bus/connection.c (bus_connections_new): refcount connection_data_slot * dbus/dbus-auth-script.c (_dbus_auth_script_run): delete unused bytes so that auth scripts pass. * bus/dispatch.c: init message_handler_slot so it gets allocated properly * bus/dispatch.c (message_handler_slot_ref): fix memleak * dbus/dbus-server-debug-pipe.c (_dbus_server_debug_pipe_new): dealloc server_pipe_hash when no longer used for benefit of leak checking * dbus/dbus-auth.c (process_command): memleak fix * bus/dispatch.c (check_hello_message): memleak fix --- diff --git a/ChangeLog b/ChangeLog index a478423..0796f2a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2003-03-17 Havoc Pennington + + All tests pass, no memleaks, no valgrind complaints. + + * bus/test.c: refcount handler_slot + + * bus/connection.c (bus_connections_new): refcount + connection_data_slot + + * dbus/dbus-auth-script.c (_dbus_auth_script_run): delete unused + bytes so that auth scripts pass. + + * bus/dispatch.c: init message_handler_slot so it gets allocated + properly + + * bus/dispatch.c (message_handler_slot_ref): fix memleak + + * dbus/dbus-server-debug-pipe.c (_dbus_server_debug_pipe_new): + dealloc server_pipe_hash when no longer used for benefit of + leak checking + + * dbus/dbus-auth.c (process_command): memleak fix + + * bus/dispatch.c (check_hello_message): memleak fix + 2003-03-16 Havoc Pennington * dbus/dbus-bus.c (ensure_bus_data): fix double-unref of the data slot diff --git a/bus/connection.c b/bus/connection.c index a861668..1b59819 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -37,6 +37,7 @@ struct BusConnections }; static int connection_data_slot = -1; +static int connection_data_slot_refcount = 0; typedef struct { @@ -51,6 +52,39 @@ typedef struct #define BUS_CONNECTION_DATA(connection) (dbus_connection_get_data ((connection), connection_data_slot)) +static dbus_bool_t +connection_data_slot_ref (void) +{ + if (connection_data_slot < 0) + { + connection_data_slot = dbus_connection_allocate_data_slot (); + + if (connection_data_slot < 0) + return FALSE; + + _dbus_assert (connection_data_slot_refcount == 0); + } + + connection_data_slot_refcount += 1; + + return TRUE; + +} + +static void +connection_data_slot_unref (void) +{ + _dbus_assert (connection_data_slot_refcount > 0); + + connection_data_slot_refcount -= 1; + + if (connection_data_slot_refcount == 0) + { + dbus_connection_free_data_slot (connection_data_slot); + connection_data_slot = -1; + } +} + void bus_connection_disconnected (DBusConnection *connection) { @@ -209,6 +243,7 @@ free_connection_data (void *data) if (d->oom_preallocated) dbus_connection_free_preallocated_send (d->connection, d->oom_preallocated); + if (d->oom_message) dbus_message_unref (d->oom_message); @@ -222,17 +257,15 @@ bus_connections_new (BusContext *context) { BusConnections *connections; - if (connection_data_slot < 0) - { - connection_data_slot = dbus_connection_allocate_data_slot (); - - if (connection_data_slot < 0) - return NULL; - } + if (!connection_data_slot_ref ()) + return NULL; connections = dbus_new0 (BusConnections, 1); if (connections == NULL) - return NULL; + { + connection_data_slot_unref (); + return NULL; + } connections->refcount = 1; connections->context = context; @@ -268,7 +301,9 @@ bus_connections_unref (BusConnections *connections) _dbus_list_clear (&connections->list); - dbus_free (connections); + dbus_free (connections); + + connection_data_slot_unref (); } } @@ -286,6 +321,8 @@ bus_connections_setup_connection (BusConnections *connections, d->connections = connections; d->connection = connection; + + _dbus_assert (connection_data_slot >= 0); if (!dbus_connection_set_data (connection, connection_data_slot, @@ -329,6 +366,11 @@ bus_connections_setup_connection (BusConnections *connections, out: if (!retval) { + if (!dbus_connection_set_data (connection, + connection_data_slot, + NULL, NULL)) + _dbus_assert_not_reached ("failed to set connection data to null"); + if (!dbus_connection_set_watch_functions (connection, NULL, NULL, NULL, connection, diff --git a/bus/dispatch.c b/bus/dispatch.c index ffb7bd9..5365a11 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -33,7 +33,7 @@ #include #include -static int message_handler_slot; +static int message_handler_slot = -1; static int message_handler_slot_refcount; typedef struct @@ -157,7 +157,7 @@ bus_dispatch (DBusConnection *connection, transaction = NULL; dbus_error_init (&error); - + context = bus_connection_get_context (connection); _dbus_assert (context != NULL); @@ -334,10 +334,15 @@ bus_dispatch_message_handler (DBusMessageHandler *handler, static dbus_bool_t message_handler_slot_ref (void) { - message_handler_slot = dbus_connection_allocate_data_slot (); - if (message_handler_slot < 0) - return FALSE; + { + message_handler_slot = dbus_connection_allocate_data_slot (); + + if (message_handler_slot < 0) + return FALSE; + + _dbus_assert (message_handler_slot_refcount == 0); + } message_handler_slot_refcount += 1; @@ -348,7 +353,9 @@ 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); @@ -356,6 +363,18 @@ message_handler_slot_unref (void) } } +static void +free_message_handler (void *data) +{ + DBusMessageHandler *handler = data; + + _dbus_assert (message_handler_slot >= 0); + _dbus_assert (message_handler_slot_refcount > 0); + + dbus_message_handler_unref (handler); + message_handler_slot_unref (); +} + dbus_bool_t bus_dispatch_add_connection (DBusConnection *connection) { @@ -369,7 +388,7 @@ bus_dispatch_add_connection (DBusConnection *connection) { message_handler_slot_unref (); return FALSE; - } + } if (!dbus_connection_add_filter (connection, handler)) { @@ -379,12 +398,14 @@ bus_dispatch_add_connection (DBusConnection *connection) return FALSE; } + _dbus_assert (message_handler_slot >= 0); + _dbus_assert (message_handler_slot_refcount > 0); + if (!dbus_connection_set_data (connection, message_handler_slot, handler, - (DBusFreeFunction)dbus_message_handler_unref)) + free_message_handler)) { - dbus_connection_remove_filter (connection, handler); dbus_message_handler_unref (handler); message_handler_slot_unref (); @@ -403,8 +424,6 @@ bus_dispatch_remove_connection (DBusConnection *connection) dbus_connection_set_data (connection, message_handler_slot, NULL, NULL); - - message_handler_slot_unref (); } #ifdef DBUS_BUILD_TESTS @@ -551,6 +570,23 @@ kill_client_connection (BusContext *context, _dbus_assert_not_reached ("stuff left in message queues after disconnecting a client"); } +static void +kill_client_connection_unchecked (DBusConnection *connection) +{ + /* This kills the connection without expecting it to affect + * the rest of the bus. + */ + _dbus_verbose ("Unchecked kill of connection %p\n", connection); + + dbus_connection_ref (connection); + dbus_connection_disconnect (connection); + /* dispatching disconnect handler will unref once */ + if (bus_connection_dispatch_one_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)); +} + typedef struct { dbus_bool_t failed; @@ -691,7 +727,10 @@ check_hello_message (BusContext *context, return TRUE; if (!dbus_connection_send (connection, message, &serial)) - return TRUE; + { + dbus_message_unref (message); + return TRUE; + } dbus_message_unref (message); message = NULL; @@ -870,15 +909,7 @@ check_hello_connection (BusContext *context) /* 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 (bus_connection_dispatch_one_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; + kill_client_connection_unchecked (connection); } else { @@ -928,6 +959,9 @@ check1_try_iterations (BusContext *context, } _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); + + _dbus_verbose ("=================\n%s: all iterations passed\n=================\n", + description); } dbus_bool_t @@ -981,20 +1015,11 @@ bus_dispatch_test (const DBusString *test_data_dir) check1_try_iterations (context, "create_and_hello", check_hello_connection); - dbus_connection_disconnect (foo); - if (bus_connection_dispatch_one_message (foo)) - _dbus_assert_not_reached ("extra message in queue"); - _dbus_assert (!bus_test_client_listed (foo)); - - dbus_connection_disconnect (bar); - if (bus_connection_dispatch_one_message (bar)) - _dbus_assert_not_reached ("extra message in queue"); - _dbus_assert (!bus_test_client_listed (bar)); + _dbus_verbose ("Disconnecting foo, bar, and baz\n"); - dbus_connection_disconnect (baz); - if (bus_connection_dispatch_one_message (baz)) - _dbus_assert_not_reached ("extra message in queue"); - _dbus_assert (!bus_test_client_listed (baz)); + kill_client_connection_unchecked (foo); + kill_client_connection_unchecked (bar); + kill_client_connection_unchecked (baz); bus_context_unref (context); diff --git a/bus/test.c b/bus/test.c index 7de1d81..bc195ef 100644 --- a/bus/test.c +++ b/bus/test.c @@ -112,6 +112,49 @@ client_disconnect_handler (DBusMessageHandler *handler, } static int handler_slot = -1; +static int handler_slot_refcount = 0; + +static dbus_bool_t +handler_slot_ref (void) +{ + if (handler_slot < 0) + { + handler_slot = dbus_connection_allocate_data_slot (); + + if (handler_slot < 0) + return FALSE; + + _dbus_assert (handler_slot_refcount == 0); + } + + handler_slot_refcount += 1; + + return TRUE; + +} + +static void +handler_slot_unref (void) +{ + _dbus_assert (handler_slot_refcount > 0); + + handler_slot_refcount -= 1; + + if (handler_slot_refcount == 0) + { + dbus_connection_free_data_slot (handler_slot); + handler_slot = -1; + } +} + +static void +free_handler (void *data) +{ + DBusMessageHandler *handler = data; + + dbus_message_handler_unref (handler); + handler_slot_unref (); +} dbus_bool_t bus_setup_debug_client (DBusConnection *connection) @@ -119,11 +162,6 @@ 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); @@ -160,12 +198,17 @@ bus_setup_debug_client (DBusConnection *connection) if (!_dbus_list_append (&clients, connection)) goto out; - /* Set up handler to be destroyed */ + if (!handler_slot_ref ()) + 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; + free_handler)) + { + handler_slot_unref (); + goto out; + } retval = TRUE; diff --git a/dbus/dbus-auth-script.c b/dbus/dbus-auth-script.c index d954c8d..3aaf568 100644 --- a/dbus/dbus-auth-script.c +++ b/dbus/dbus-auth-script.c @@ -541,6 +541,7 @@ _dbus_auth_script_run (const DBusString *filename) if (_dbus_string_equal (&expected, unused)) { + _dbus_auth_delete_unused_bytes (auth); _dbus_string_free (&expected); } else diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c index 9e2b1d9..8f8aec4 100644 --- a/dbus/dbus-auth.c +++ b/dbus/dbus-auth.c @@ -1665,6 +1665,7 @@ process_command (DBusAuth *auth) if (!_dbus_string_init (&args, _DBUS_INT_MAX)) { + _dbus_string_free (&command); auth->needed_memory = TRUE; return FALSE; } diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index ed677e7..d70ff71 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -866,6 +866,8 @@ _dbus_connection_last_unref (DBusConnection *connection) DBusHashIter iter; DBusList *link; + _dbus_verbose ("Finalizing connection %p\n", connection); + _dbus_assert (connection->refcount == 0); /* You have to disconnect the connection before unref:ing it. Otherwise @@ -969,6 +971,10 @@ dbus_connection_unref (DBusConnection *connection) connection->refcount -= 1; last_unref = (connection->refcount == 0); +#if 0 + _dbus_verbose ("unref() connection %p count = %d\n", connection, connection->refcount); +#endif + dbus_mutex_unlock (connection->mutex); if (last_unref) diff --git a/dbus/dbus-dataslot.c b/dbus/dbus-dataslot.c index 53fb9e4..46e2bfc 100644 --- a/dbus/dbus-dataslot.c +++ b/dbus/dbus-dataslot.c @@ -57,6 +57,10 @@ _dbus_data_slot_allocator_init (DBusDataSlotAllocator *allocator) * Allocates an integer ID to be used for storing data * in a #DBusDataSlotList. * + * @todo all over the code we have foo_slot and foo_slot_refcount, + * would be better to add an interface for that to + * DBusDataSlotAllocator so it isn't cut-and-pasted everywhere. + * * @param allocator the allocator * @returns the integer ID, or -1 on failure */ @@ -103,6 +107,9 @@ _dbus_data_slot_allocator_alloc (DBusDataSlotAllocator *allocator) _dbus_assert (slot >= 0); _dbus_assert (slot < allocator->n_allocated_slots); + + _dbus_verbose ("Allocated slot %d on allocator %p total %d slots allocated %d used\n", + slot, allocator, allocator->n_allocated_slots, allocator->n_used_slots); out: dbus_mutex_unlock (allocator->lock); @@ -137,6 +144,9 @@ _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator, allocator->allocated_slots = NULL; allocator->n_allocated_slots = 0; } + + _dbus_verbose ("Freed slot %d on allocator %p total %d allocated %d used\n", + slot, allocator, allocator->n_allocated_slots, allocator->n_used_slots); dbus_mutex_unlock (allocator->lock); } @@ -243,6 +253,7 @@ _dbus_data_slot_list_get (DBusDataSlotAllocator *allocator, */ if (!dbus_mutex_lock (allocator->lock)) return FALSE; + _dbus_assert (slot >= 0); _dbus_assert (slot < allocator->n_allocated_slots); _dbus_assert (allocator->allocated_slots[slot] == slot); dbus_mutex_unlock (allocator->lock); diff --git a/dbus/dbus-server-debug-pipe.c b/dbus/dbus-server-debug-pipe.c index 46e78dd..c9a2502 100644 --- a/dbus/dbus-server-debug-pipe.c +++ b/dbus/dbus-server-debug-pipe.c @@ -59,13 +59,48 @@ struct DBusServerDebugPipe dbus_bool_t disconnected; /**< TRUE if disconnect has been called */ }; +/* FIXME not threadsafe (right now the test suite doesn't use threads anyhow ) */ static DBusHashTable *server_pipe_hash; +static int server_pipe_hash_refcount = 0; +static dbus_bool_t +pipe_hash_ref (void) +{ + if (!server_pipe_hash) + { + _dbus_assert (server_pipe_hash_refcount == 0); + + server_pipe_hash = _dbus_hash_table_new (DBUS_HASH_STRING, NULL, NULL); + + if (!server_pipe_hash) + return FALSE; + } + + server_pipe_hash_refcount = 1; + + return TRUE; +} + +static void +pipe_hash_unref (void) +{ + _dbus_assert (server_pipe_hash != NULL); + _dbus_assert (server_pipe_hash_refcount > 0); + + server_pipe_hash_refcount -= 1; + if (server_pipe_hash_refcount == 0) + { + _dbus_hash_table_unref (server_pipe_hash); + server_pipe_hash = NULL; + } +} static void debug_finalize (DBusServer *server) { DBusServerDebugPipe *debug_server = (DBusServerDebugPipe*) server; + + pipe_hash_unref (); _dbus_server_finalize_base (server); @@ -107,27 +142,23 @@ _dbus_server_debug_pipe_new (const char *server_name, { DBusServerDebugPipe *debug_server; - if (!server_pipe_hash) - { - server_pipe_hash = _dbus_hash_table_new (DBUS_HASH_STRING, NULL, NULL); - - if (!server_pipe_hash) - { - dbus_set_result (result, DBUS_RESULT_NO_MEMORY); - return NULL; - } - } - + if (!pipe_hash_ref ()) + return NULL; + if (_dbus_hash_table_lookup_string (server_pipe_hash, server_name) != NULL) { dbus_set_result (result, DBUS_RESULT_ADDRESS_IN_USE); + pipe_hash_unref (); return NULL; } debug_server = dbus_new0 (DBusServerDebugPipe, 1); if (debug_server == NULL) - return NULL; + { + pipe_hash_unref (); + return NULL; + } debug_server->name = _dbus_strdup (server_name); if (debug_server->name == NULL) @@ -136,6 +167,9 @@ _dbus_server_debug_pipe_new (const char *server_name, dbus_free (debug_server); dbus_set_result (result, DBUS_RESULT_NO_MEMORY); + + pipe_hash_unref (); + return NULL; } if (!_dbus_server_init_base (&debug_server->base, @@ -146,6 +180,7 @@ _dbus_server_debug_pipe_new (const char *server_name, dbus_set_result (result, DBUS_RESULT_NO_MEMORY); + pipe_hash_unref (); return NULL; } @@ -159,6 +194,7 @@ _dbus_server_debug_pipe_new (const char *server_name, dbus_set_result (result, DBUS_RESULT_NO_MEMORY); + pipe_hash_unref (); return NULL; }