Commit d2d97a21 authored by David Zeuthen's avatar David Zeuthen

Bug 625584 – Crashes application on unref with signal subscription

Don't do too much work in the finalizer - in particular, there's no
need to send RemoveMatch() messages to the bus daemon since we're
going to sever the connection and the bus will garbage collect
anyway. In this case it crashed the process.

Also add a test case that checks that the appropriate GDestroyNotify
callbacks are called when unreffing a connection with either 1)
exported objects; 2) signal subscriptions or 3) filter functions
.. yes, ideally apps would unregister such callbacks before giving up
their ref but that's not how things work :-)
Signed-off-by: default avatarDavid Zeuthen <davidz@redhat.com>
parent 1b5b87bf
......@@ -318,6 +318,9 @@ struct _GDBusConnection
GDBusAuthObserver *authentication_observer;
GCredentials *crendentials;
/* set to TRUE when finalizing */
gboolean finalizing;
};
typedef struct ExportedObject ExportedObject;
......@@ -408,13 +411,19 @@ g_dbus_connection_finalize (GObject *object)
{
GDBusConnection *connection = G_DBUS_CONNECTION (object);
connection->finalizing = TRUE;
purge_all_signal_subscriptions (connection);
purge_all_filters (connection);
g_ptr_array_unref (connection->filters);
if (connection->authentication_observer != NULL)
g_object_unref (connection->authentication_observer);
if (connection->auth != NULL)
g_object_unref (connection->auth);
//g_debug ("finalizing %p", connection);
if (connection->stream != NULL)
{
/* We don't really care if closing the stream succeeds or not */
......@@ -437,7 +446,6 @@ g_dbus_connection_finalize (GObject *object)
g_hash_table_unref (connection->map_method_serial_to_send_message_data);
purge_all_signal_subscriptions (connection);
g_hash_table_unref (connection->map_rule_to_signal_data);
g_hash_table_unref (connection->map_id_to_signal_data);
g_hash_table_unref (connection->map_sender_unique_name_to_signal_data_array);
......@@ -447,9 +455,6 @@ g_dbus_connection_finalize (GObject *object)
g_hash_table_unref (connection->map_id_to_es);
g_hash_table_unref (connection->map_object_path_to_es);
purge_all_filters (connection);
g_ptr_array_unref (connection->filters);
if (connection->main_context_at_construction != NULL)
g_main_context_unref (connection->main_context_at_construction);
......@@ -3047,7 +3052,7 @@ g_dbus_connection_signal_subscribe (GDBusConnection *connection,
/* ---------------------------------------------------------------------------------------------------- */
/* must hold lock when calling this */
/* must hold lock when calling this (except if connection->finalizing is TRUE) */
static void
unsubscribe_id_internal (GDBusConnection *connection,
guint subscription_id,
......@@ -3097,7 +3102,7 @@ unsubscribe_id_internal (GDBusConnection *connection,
if (connection->flags & G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION)
{
if (!is_signal_data_for_name_lost_or_acquired (signal_data))
if (!connection->closed)
if (!connection->closed && !connection->finalizing)
remove_match_rule (connection, signal_data->rule);
}
signal_data_free (signal_data);
......
......@@ -32,10 +32,65 @@
/* all tests rely on a shared mainloop */
static GMainLoop *loop = NULL;
static gboolean
test_connection_quit_mainloop (gpointer user_data)
{
gboolean *quit_mainloop_fired = user_data;
*quit_mainloop_fired = TRUE;
g_main_loop_quit (loop);
return TRUE;
}
/* ---------------------------------------------------------------------------------------------------- */
/* Connection life-cycle testing */
/* ---------------------------------------------------------------------------------------------------- */
static const GDBusInterfaceInfo boo_interface_info =
{
-1,
"org.example.Boo",
(GDBusMethodInfo **) NULL,
(GDBusSignalInfo **) NULL,
(GDBusPropertyInfo **) NULL,
NULL,
};
static const GDBusInterfaceVTable boo_vtable =
{
NULL, /* _method_call */
NULL, /* _get_property */
NULL /* _set_property */
};
static gboolean
some_filter_func (GDBusConnection *connection,
GDBusMessage *message,
gboolean incoming,
gpointer user_data)
{
return FALSE;
}
static void
on_name_owner_changed (GDBusConnection *connection,
const gchar *sender_name,
const gchar *object_path,
const gchar *interface_name,
const gchar *signal_name,
GVariant *parameters,
gpointer user_data)
{
}
static void
a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop (gpointer user_data)
{
gboolean *val = user_data;
*val = TRUE;
g_main_loop_quit (loop);
}
static void
test_connection_life_cycle (void)
{
......@@ -43,6 +98,12 @@ test_connection_life_cycle (void)
GDBusConnection *c;
GDBusConnection *c2;
GError *error;
gboolean on_signal_registration_freed_called;
gboolean on_filter_freed_called;
gboolean on_register_object_freed_called;
gboolean quit_mainloop_fired;
guint quit_mainloop_id;
guint registration_id;
error = NULL;
......@@ -99,6 +160,66 @@ test_connection_life_cycle (void)
g_assert (!ret);
g_object_unref (c2);
/*
* Check that the finalization code works
*
* (and that the GDestroyNotify for filters and objects and signal
* registrations are run as expected)
*/
error = NULL;
c2 = _g_bus_get_priv (G_BUS_TYPE_SESSION, NULL, &error);
g_assert_no_error (error);
g_assert (c2 != NULL);
/* signal registration */
on_signal_registration_freed_called = FALSE;
g_dbus_connection_signal_subscribe (c2,
"org.freedesktop.DBus", /* bus name */
"org.freedesktop.DBus", /* interface */
"NameOwnerChanged", /* member */
"/org/freesktop/DBus", /* path */
NULL, /* arg0 */
G_DBUS_SIGNAL_FLAGS_NONE,
on_name_owner_changed,
&on_signal_registration_freed_called,
a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop);
/* filter func */
on_filter_freed_called = FALSE;
g_dbus_connection_add_filter (c2,
some_filter_func,
&on_filter_freed_called,
a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop);
/* object registration */
on_register_object_freed_called = FALSE;
error = NULL;
registration_id = g_dbus_connection_register_object (c2,
"/foo",
(GDBusInterfaceInfo *) &boo_interface_info,
&boo_vtable,
&on_register_object_freed_called,
a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop,
&error);
g_assert_no_error (error);
g_assert (registration_id > 0);
/* ok, finalize the connection and check that all the GDestroyNotify functions are invoked as expected */
g_object_unref (c2);
quit_mainloop_fired = FALSE;
quit_mainloop_id = g_timeout_add (1000, test_connection_quit_mainloop, &quit_mainloop_fired);
while (TRUE)
{
if (on_signal_registration_freed_called &&
on_filter_freed_called &&
on_register_object_freed_called)
break;
if (quit_mainloop_fired)
break;
g_main_loop_run (loop);
}
g_source_remove (quit_mainloop_id);
g_assert (on_signal_registration_freed_called);
g_assert (on_filter_freed_called);
g_assert (on_register_object_freed_called);
g_assert (!quit_mainloop_fired);
/*
* Check for correct behavior when the bus goes away
*
......@@ -353,15 +474,6 @@ test_connection_signal_handler (GDBusConnection *connection,
g_main_loop_quit (loop);
}
static gboolean
test_connection_signal_quit_mainloop (gpointer user_data)
{
gboolean *quit_mainloop_fired = user_data;
*quit_mainloop_fired = TRUE;
g_main_loop_quit (loop);
return TRUE;
}
static void
test_connection_signals (void)
{
......@@ -539,7 +651,7 @@ test_connection_signals (void)
gboolean quit_mainloop_fired;
guint quit_mainloop_id;
quit_mainloop_fired = FALSE;
quit_mainloop_id = g_timeout_add (5000, test_connection_signal_quit_mainloop, &quit_mainloop_fired);
quit_mainloop_id = g_timeout_add (5000, test_connection_quit_mainloop, &quit_mainloop_fired);
while (count_name_owner_changed < 2 && !quit_mainloop_fired)
g_main_loop_run (loop);
g_source_remove (quit_mainloop_id);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment