Commit 7036415c authored by David Zeuthen's avatar David Zeuthen

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: default avatarDavid Zeuthen <davidz@redhat.com>
parent 919f64ac
...@@ -180,6 +180,77 @@ static GDBusConnection *the_system_bus = NULL; ...@@ -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: <internal>
* @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 static gboolean
_g_strv_has_string (const gchar* const *haystack, _g_strv_has_string (const gchar* const *haystack,
const gchar *needle) const gchar *needle)
...@@ -3234,8 +3305,9 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection, ...@@ -3234,8 +3305,9 @@ g_dbus_connection_signal_unsubscribe (GDBusConnection *connection,
{ {
SignalSubscriber *subscriber; SignalSubscriber *subscriber;
subscriber = &(g_array_index (subscribers, SignalSubscriber, n)); subscriber = &(g_array_index (subscribers, SignalSubscriber, n));
if (subscriber->user_data_free_func != NULL) call_destroy_notify (subscriber->context,
subscriber->user_data_free_func (subscriber->user_data); subscriber->user_data_free_func,
subscriber->user_data);
if (subscriber->context != NULL) if (subscriber->context != NULL)
g_main_context_unref (subscriber->context); g_main_context_unref (subscriber->context);
} }
...@@ -3481,8 +3553,9 @@ purge_all_signal_subscriptions (GDBusConnection *connection) ...@@ -3481,8 +3553,9 @@ purge_all_signal_subscriptions (GDBusConnection *connection)
{ {
SignalSubscriber *subscriber; SignalSubscriber *subscriber;
subscriber = &(g_array_index (subscribers, SignalSubscriber, n)); subscriber = &(g_array_index (subscribers, SignalSubscriber, n));
if (subscriber->user_data_free_func != NULL) call_destroy_notify (subscriber->context,
subscriber->user_data_free_func (subscriber->user_data); subscriber->user_data_free_func,
subscriber->user_data);
if (subscriber->context != NULL) if (subscriber->context != NULL)
g_main_context_unref (subscriber->context); g_main_context_unref (subscriber->context);
} }
...@@ -3564,9 +3637,9 @@ exported_interface_free (ExportedInterface *ei) ...@@ -3564,9 +3637,9 @@ exported_interface_free (ExportedInterface *ei)
{ {
g_dbus_interface_info_unref ((GDBusInterfaceInfo *) ei->interface_info); g_dbus_interface_info_unref ((GDBusInterfaceInfo *) ei->interface_info);
if (ei->user_data_free_func != NULL) call_destroy_notify (ei->context,
/* TODO: push to thread-default mainloop */ ei->user_data_free_func,
ei->user_data_free_func (ei->user_data); ei->user_data);
if (ei->context != NULL) if (ei->context != NULL)
g_main_context_unref (ei->context); g_main_context_unref (ei->context);
...@@ -5249,9 +5322,9 @@ struct ExportedSubtree ...@@ -5249,9 +5322,9 @@ struct ExportedSubtree
static void static void
exported_subtree_free (ExportedSubtree *es) exported_subtree_free (ExportedSubtree *es)
{ {
if (es->user_data_free_func != NULL) call_destroy_notify (es->context,
/* TODO: push to thread-default mainloop */ es->user_data_free_func,
es->user_data_free_func (es->user_data); es->user_data);
if (es->context != NULL) if (es->context != NULL)
g_main_context_unref (es->context); g_main_context_unref (es->context);
......
...@@ -32,10 +32,36 @@ ...@@ -32,10 +32,36 @@
/* all tests rely on a shared mainloop */ /* all tests rely on a shared mainloop */
static GMainLoop *loop = NULL; 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 static gboolean
test_connection_quit_mainloop (gpointer user_data) 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; *quit_mainloop_fired = TRUE;
g_main_loop_quit (loop); g_main_loop_quit (loop);
return TRUE; return TRUE;
...@@ -85,9 +111,9 @@ on_name_owner_changed (GDBusConnection *connection, ...@@ -85,9 +111,9 @@ on_name_owner_changed (GDBusConnection *connection,
static void static void
a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop (gpointer user_data) 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; *val = TRUE;
//_log ("destroynotify fired for %p", val);
g_main_loop_quit (loop); g_main_loop_quit (loop);
} }
...@@ -98,10 +124,10 @@ test_connection_life_cycle (void) ...@@ -98,10 +124,10 @@ test_connection_life_cycle (void)
GDBusConnection *c; GDBusConnection *c;
GDBusConnection *c2; GDBusConnection *c2;
GError *error; GError *error;
gboolean on_signal_registration_freed_called; volatile gboolean on_signal_registration_freed_called;
gboolean on_filter_freed_called; volatile gboolean on_filter_freed_called;
gboolean on_register_object_freed_called; volatile gboolean on_register_object_freed_called;
gboolean quit_mainloop_fired; volatile gboolean quit_mainloop_fired;
guint quit_mainloop_id; guint quit_mainloop_id;
guint registration_id; guint registration_id;
...@@ -182,13 +208,13 @@ test_connection_life_cycle (void) ...@@ -182,13 +208,13 @@ test_connection_life_cycle (void)
NULL, /* arg0 */ NULL, /* arg0 */
G_DBUS_SIGNAL_FLAGS_NONE, G_DBUS_SIGNAL_FLAGS_NONE,
on_name_owner_changed, 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); a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop);
/* filter func */ /* filter func */
on_filter_freed_called = FALSE; on_filter_freed_called = FALSE;
g_dbus_connection_add_filter (c2, g_dbus_connection_add_filter (c2,
some_filter_func, some_filter_func,
&on_filter_freed_called, (gpointer) &on_filter_freed_called,
a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop); a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop);
/* object registration */ /* object registration */
on_register_object_freed_called = FALSE; on_register_object_freed_called = FALSE;
...@@ -197,7 +223,7 @@ test_connection_life_cycle (void) ...@@ -197,7 +223,7 @@ test_connection_life_cycle (void)
"/foo", "/foo",
(GDBusInterfaceInfo *) &boo_interface_info, (GDBusInterfaceInfo *) &boo_interface_info,
&boo_vtable, &boo_vtable,
&on_register_object_freed_called, (gpointer) &on_register_object_freed_called,
a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop, a_gdestroynotify_that_sets_a_gboolean_to_true_and_quits_loop,
&error); &error);
g_assert_no_error (error); g_assert_no_error (error);
...@@ -205,7 +231,14 @@ test_connection_life_cycle (void) ...@@ -205,7 +231,14 @@ test_connection_life_cycle (void)
/* ok, finalize the connection and check that all the GDestroyNotify functions are invoked as expected */ /* ok, finalize the connection and check that all the GDestroyNotify functions are invoked as expected */
g_object_unref (c2); g_object_unref (c2);
quit_mainloop_fired = FALSE; 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) while (TRUE)
{ {
if (on_signal_registration_freed_called && if (on_signal_registration_freed_called &&
...@@ -214,7 +247,9 @@ test_connection_life_cycle (void) ...@@ -214,7 +247,9 @@ test_connection_life_cycle (void)
break; break;
if (quit_mainloop_fired) if (quit_mainloop_fired)
break; break;
//_log ("entering loop");
g_main_loop_run (loop); g_main_loop_run (loop);
//_log ("exiting loop");
} }
g_source_remove (quit_mainloop_id); g_source_remove (quit_mainloop_id);
g_assert (on_signal_registration_freed_called); g_assert (on_signal_registration_freed_called);
......
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