From 227376fbd9b7c4f370b64dd0b6daa2c62e0e95e0 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 13 May 2011 11:28:49 +0100 Subject: [PATCH 4/7] DBusMessage: provide a hook to allow refcounting to be traced This has several functions: * when under gdb, provide a function which can be used in breakpoints * when not under valgrind and DBUS_MESSAGE_TRACE=1 is set, emit a _dbus_verbose when a message's refcount changes * when under valgrind and DBUS_MESSAGE_TRACE=1 is set, emit a VALGRIND_PRINTF_BACKTRACE when a message's refcount changes, which lets you see the complete history of each message to track down reference leaks In addition to refcount changes, we also trace on ownership changes that don't change the refcount. Compile-time support is currently conditional on DBUS_ENABLE_VERBOSE_MODE, but could be separated out if desired. --- dbus/dbus-connection.c | 23 ++++++++++++++- dbus/dbus-message-internal.h | 9 ++++++ dbus/dbus-message.c | 62 +++++++++++++++++++++++++++++++++++------- dbus/dbus-pending-call.c | 4 ++- 4 files changed, 85 insertions(+), 13 deletions(-) diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index d80604a..c08c70f 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -520,7 +520,11 @@ _dbus_connection_queue_received_message_link (DBusConnection *connection, dbus_message_get_signature (message), dbus_message_get_reply_serial (message), connection, - connection->n_incoming);} + connection->n_incoming); + + _dbus_message_trace_ref (message, -1, -1, + "_dbus_conection_queue_received_message_link"); +} /** * Adds a link + message to the incoming message queue. @@ -541,7 +545,10 @@ _dbus_connection_queue_synthesized_message_link (DBusConnection *connection, connection->n_incoming += 1; _dbus_connection_wakeup_mainloop (connection); - + + _dbus_message_trace_ref (link->data, -1, -1, + "_dbus_connection_queue_synthesized_message_link"); + _dbus_verbose ("Synthesized message %p added to incoming queue %p, %d incoming\n", link->data, connection, connection->n_incoming); } @@ -3835,6 +3842,8 @@ dbus_connection_borrow_message (DBusConnection *connection) CONNECTION_UNLOCK (connection); + _dbus_message_trace_ref (message, -1, -1, "dbus_connection_borrow_message"); + /* We don't update dispatch status until it's returned or stolen */ return message; @@ -3869,6 +3878,8 @@ dbus_connection_return_message (DBusConnection *connection, status = _dbus_connection_get_dispatch_status_unlocked (connection); _dbus_connection_update_dispatch_status_and_unlock (connection, status); + + _dbus_message_trace_ref (message, -1, -1, "dbus_connection_return_message"); } /** @@ -3910,6 +3921,8 @@ dbus_connection_steal_borrowed_message (DBusConnection *connection, status = _dbus_connection_get_dispatch_status_unlocked (connection); _dbus_connection_update_dispatch_status_and_unlock (connection, status); + _dbus_message_trace_ref (message, -1, -1, + "dbus_connection_steal_borrowed_message"); } /* See dbus_connection_pop_message, but requires the caller to own @@ -3944,6 +3957,9 @@ _dbus_connection_pop_message_link_unlocked (DBusConnection *connection) dbus_message_get_signature (link->data), connection, connection->n_incoming); + _dbus_message_trace_ref (link->data, -1, -1, + "_dbus_connection_pop_message_link_unlocked"); + check_disconnected_message_arrived_unlocked (connection, link->data); return link; @@ -4005,6 +4021,9 @@ _dbus_connection_putback_message_link_unlocked (DBusConnection *connection, "no member", dbus_message_get_signature (message_link->data), connection, connection->n_incoming); + + _dbus_message_trace_ref (message_link->data, -1, -1, + "_dbus_connection_putback_message_link_unlocked"); } /** diff --git a/dbus/dbus-message-internal.h b/dbus/dbus-message-internal.h index 870934b..660e840 100644 --- a/dbus/dbus-message-internal.h +++ b/dbus/dbus-message-internal.h @@ -30,6 +30,15 @@ DBUS_BEGIN_DECLS +#ifdef DBUS_ENABLE_VERBOSE_MODE +void _dbus_message_trace_ref (DBusMessage *message, + int old_refcount, + int new_refcount, + const char *why); +#else +#define _dbus_message_trace_ref(m,o,n,w) do {} while (0) +#endif + typedef struct DBusMessageLoader DBusMessageLoader; void _dbus_message_get_network_data (DBusMessage *message, diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 0855951..eebcc85 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -34,6 +34,7 @@ #include "dbus-memory.h" #include "dbus-list.h" #include "dbus-threads-internal.h" +#include "dbus-valgrind-internal.h" #ifdef HAVE_UNIX_FD_PASSING #include "dbus-sysdeps-unix.h" #endif @@ -83,6 +84,45 @@ _dbus_enable_message_cache (void) # define _dbus_enable_message_cache() (TRUE) #endif +#ifndef _dbus_message_trace_ref +void +_dbus_message_trace_ref (DBusMessage *message, + int old_refcount, + int new_refcount, + const char *why) +{ + static int enabled = -1; + + if (enabled < 0) + { + const char *s = _dbus_getenv ("DBUS_MESSAGE_TRACE"); + + enabled = FALSE; + + if (s && *s) + { + if (*s == '0') + enabled = FALSE; + else if (*s == '1') + enabled = TRUE; + else + _dbus_warn ("DBUS_MESSAGE_TRACE should be 0 or 1 if set, not '%s'", + s); + } + } + + if (enabled) + { + if (RUNNING_ON_VALGRIND) + VALGRIND_PRINTF_BACKTRACE ("DBusMessage %p %d -> %d refs (%s)", + message, old_refcount, new_refcount, why); + else + _dbus_verbose ("DBusMessage %p %d -> %d refs (%s)", + message, old_refcount, new_refcount, why); + } +} +#endif + /* Not thread locked, but strictly const/read-only so should be OK */ /** An static string representing an empty signature */ @@ -1169,6 +1209,8 @@ dbus_message_new_empty_header (void) } #endif + _dbus_message_trace_ref (message, 0, 1, "new_empty_header"); + message->byte_order = DBUS_COMPILER_BYTE_ORDER; message->locked = FALSE; #ifndef DBUS_DISABLE_CHECKS @@ -1571,6 +1613,7 @@ dbus_message_copy (const DBusMessage *message) #endif + _dbus_message_trace_ref (retval, 0, 1, "copy"); return retval; failed_copy: @@ -1598,20 +1641,17 @@ dbus_message_copy (const DBusMessage *message) DBusMessage * dbus_message_ref (DBusMessage *message) { + dbus_int32_t old_refcount; + _dbus_return_val_if_fail (message != NULL, NULL); _dbus_return_val_if_fail (message->generation == _dbus_current_generation, NULL); _dbus_return_val_if_fail (!message->in_cache, NULL); -#ifdef DBUS_DISABLE_ASSERT - _dbus_atomic_inc (&message->refcount); -#else - { - dbus_int32_t old_refcount; - - old_refcount = _dbus_atomic_inc (&message->refcount); - _dbus_assert (old_refcount >= 1); - } -#endif + old_refcount = _dbus_atomic_inc (&message->refcount); + _dbus_assert (old_refcount >= 1); + _dbus_message_trace_ref (message, old_refcount, old_refcount + 1, "ref"); + /* unused if neither asserting nor tracing */ + (void) old_refcount; return message; } @@ -1636,6 +1676,8 @@ dbus_message_unref (DBusMessage *message) _dbus_assert (old_refcount >= 1); + _dbus_message_trace_ref (message, old_refcount, old_refcount - 1, "unref"); + if (old_refcount == 1) { /* Calls application callbacks! */ diff --git a/dbus/dbus-pending-call.c b/dbus/dbus-pending-call.c index cfb2baf..5eeb620 100644 --- a/dbus/dbus-pending-call.c +++ b/dbus/dbus-pending-call.c @@ -24,6 +24,7 @@ #include #include "dbus-internals.h" #include "dbus-connection-internal.h" +#include "dbus-message-internal.h" #include "dbus-pending-call-internal.h" #include "dbus-pending-call.h" #include "dbus-list.h" @@ -678,7 +679,8 @@ dbus_pending_call_steal_reply (DBusPendingCall *pending) pending->reply = NULL; CONNECTION_UNLOCK (pending->connection); - + + _dbus_message_trace_ref (message, -1, -1, "dbus_pending_call_steal_reply"); return message; } -- 1.7.5.4