From 7036415cc1a32bbd9cc08e516196dbd704f8b5eb Mon Sep 17 00:00:00 2001 From: David Zeuthen Date: Thu, 23 Sep 2010 17:23:30 -0400 Subject: [PATCH] GDBusConnection: Use correct GMainContext when invoking free functions Without this fix, the ./gdbus-connection test case occasionally fails, see https://bugzilla.gnome.org/show_bug.cgi?id=629945#c5 like this /gdbus/connection/basic: OK /gdbus/connection/life-cycle: ** ERROR:gdbus-connection.c:223:test_connection_life_cycle: assertion failed: (!quit_mainloop_fired) cleaning up bus with pid 21794 Aborted (core dumped) because the callback didn't happen on the same thread as where we are running the loop. Signed-off-by: David Zeuthen --- gio/gdbusconnection.c | 93 ++++++++++++++++++++++++++++++++---- gio/tests/gdbus-connection.c | 57 +++++++++++++++++----- 2 files changed, 129 insertions(+), 21 deletions(-) diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index 6a1cc9ec0..da2b9f997 100644 --- a/gio/gdbusconnection.c +++ b/gio/gdbusconnection.c @@ -180,6 +180,77 @@ static GDBusConnection *the_system_bus = NULL; /* ---------------------------------------------------------------------------------------------------- */ +typedef struct +{ + GDestroyNotify callback; + gpointer user_data; + GMainContext *context; +} CallDestroyNotifyData; + +static gboolean +call_destroy_notify_data_in_idle (gpointer user_data) +{ + CallDestroyNotifyData *data = user_data; + data->callback (data->user_data); + return FALSE; +} + +static void +call_destroy_notify_data_free (CallDestroyNotifyData *data) +{ + if (data->context != NULL) + g_main_context_unref (data->context); + g_free (data); +} + +/* + * call_destroy_notify: + * @context: A #GMainContext or %NULL. + * @callback: A #GDestroyNotify or %NULL. + * @user_data: Data to pass to @callback. + * + * Schedules @callback to run in @context. + */ +static void +call_destroy_notify (GMainContext *context, + GDestroyNotify callback, + gpointer user_data) +{ + if (callback == NULL) + goto out; + + if (context == g_main_context_get_thread_default ()) + { + callback (user_data); + } + else + { + GSource *idle_source; + CallDestroyNotifyData *data; + + data = g_new0 (CallDestroyNotifyData, 1); + data->callback = callback; + data->user_data = user_data; + data->context = context; + if (data->context != NULL) + g_main_context_ref (data->context); + + idle_source = g_idle_source_new (); + g_source_set_priority (idle_source, G_PRIORITY_DEFAULT); + g_source_set_callback (idle_source, + call_destroy_notify_data_in_idle, + data, + (GDestroyNotify) call_destroy_notify_data_free); + g_source_attach (idle_source, data->context); + g_source_unref (idle_source); + } + + out: + ; +} + +/* ---------------------------------------------------------------------------------------------------- */ + static gboolean _g_strv_has_string (const gchar* const *haystack, const gchar *needle) @@ -3234,8 +3305,9 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection, { SignalSubscriber *subscriber; subscriber = &(g_array_index (subscribers, SignalSubscriber, n)); - if (subscriber->user_data_free_func != NULL) - subscriber->user_data_free_func (subscriber->user_data); + call_destroy_notify (subscriber->context, + subscriber->user_data_free_func, + subscriber->user_data); if (subscriber->context != NULL) g_main_context_unref (subscriber->context); } @@ -3481,8 +3553,9 @@ purge_all_signal_subscriptions (GDBusConnection *connection) { SignalSubscriber *subscriber; subscriber = &(g_array_index (subscribers, SignalSubscriber, n)); - if (subscriber->user_data_free_func != NULL) - subscriber->user_data_free_func (subscriber->user_data); + call_destroy_notify (subscriber->context, + subscriber->user_data_free_func, + subscriber->user_data); if (subscriber->context != NULL) g_main_context_unref (subscriber->context); } @@ -3564,9 +3637,9 @@ exported_interface_free (ExportedInterface *ei) { g_dbus_interface_info_unref ((GDBusInterfaceInfo *) ei->interface_info); - if (ei->user_data_free_func != NULL) - /* TODO: push to thread-default mainloop */ - ei->user_data_free_func (ei->user_data); + call_destroy_notify (ei->context, + ei->user_data_free_func, + ei->user_data); if (ei->context != NULL) g_main_context_unref (ei->context); @@ -5249,9 +5322,9 @@ struct ExportedSubtree static void exported_subtree_free (ExportedSubtree *es) { - if (es->user_data_free_func != NULL) - /* TODO: push to thread-default mainloop */ - es->user_data_free_func (es->user_data); + call_destroy_notify (es->context, + es->user_data_free_func, + es->user_data); if (es->context != NULL) g_main_context_unref (es->context); diff --git a/gio/tests/gdbus-connection.c b/gio/tests/gdbus-connection.c index e74474d3e..866e27cec 100644 --- a/gio/tests/gdbus-connection.c +++ b/gio/tests/gdbus-connection.c @@ -32,10 +32,36 @@ /* all tests rely on a shared mainloop */ static GMainLoop *loop = NULL; +G_GNUC_UNUSED static void +_log (const gchar *format, ...) +{ + GTimeVal now; + time_t now_time; + struct tm *now_tm; + gchar time_buf[128]; + gchar *str; + va_list var_args; + + va_start (var_args, format); + str = g_strdup_vprintf (format, var_args); + va_end (var_args); + + g_get_current_time (&now); + now_time = (time_t) now.tv_sec; + now_tm = localtime (&now_time); + strftime (time_buf, sizeof time_buf, "%H:%M:%S", now_tm); + + g_print ("%s.%06d: %s\n", + time_buf, (gint) now.tv_usec / 1000, + str); + g_free (str); +} + static gboolean test_connection_quit_mainloop (gpointer user_data) { - gboolean *quit_mainloop_fired = user_data; + volatile gboolean *quit_mainloop_fired = user_data; + //_log ("quit_mainloop_fired"); *quit_mainloop_fired = TRUE; g_main_loop_quit (loop); return TRUE; @@ -85,9 +111,9 @@ on_name_owner_changed (GDBusConnection *connection, static void a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop (gpointer user_data) { - gboolean *val = user_data; + volatile gboolean *val = user_data; *val = TRUE; - + //_log ("destroynotify fired for %p", val); g_main_loop_quit (loop); } @@ -98,10 +124,10 @@ 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; + volatile gboolean on_signal_registration_freed_called; + volatile gboolean on_filter_freed_called; + volatile gboolean on_register_object_freed_called; + volatile gboolean quit_mainloop_fired; guint quit_mainloop_id; guint registration_id; @@ -182,13 +208,13 @@ test_connection_life_cycle (void) NULL, /* arg0 */ G_DBUS_SIGNAL_FLAGS_NONE, on_name_owner_changed, - &on_signal_registration_freed_called, + (gpointer) &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, + (gpointer) &on_filter_freed_called, a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop); /* object registration */ on_register_object_freed_called = FALSE; @@ -197,7 +223,7 @@ test_connection_life_cycle (void) "/foo", (GDBusInterfaceInfo *) &boo_interface_info, &boo_vtable, - &on_register_object_freed_called, + (gpointer) &on_register_object_freed_called, a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop, &error); g_assert_no_error (error); @@ -205,7 +231,14 @@ test_connection_life_cycle (void) /* 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 (30000, test_connection_quit_mainloop, &quit_mainloop_fired); + quit_mainloop_id = g_timeout_add (30000, test_connection_quit_mainloop, (gpointer) &quit_mainloop_fired); + //_log ("destroynotifies for\n" + // " register_object %p\n" + // " filter %p\n" + // " signal %p", + // &on_register_object_freed_called, + // &on_filter_freed_called, + // &on_signal_registration_freed_called); while (TRUE) { if (on_signal_registration_freed_called && @@ -214,7 +247,9 @@ test_connection_life_cycle (void) break; if (quit_mainloop_fired) break; + //_log ("entering loop"); g_main_loop_run (loop); + //_log ("exiting loop"); } g_source_remove (quit_mainloop_id); g_assert (on_signal_registration_freed_called);