|
Hello,
As far as i knew, the GDBusProxy object initialization flow have two mainly
actions:
(1). Listen for properties change signal to update property cache in GDBusProxy. (2). Called the GetAll metod call to get the default value of an object properties.
I've encountered a race condition with GDBusProxy object initialization. if the signal of properties_change occurred while the GDBusProxy was already in a GetAll reply handler flow,
the properties_change signal will be immediately discarded. This causes the GDBusProxy object has not yet initialized when the callback function on_properties_changed() called.
The ascii figure below shows the possible execution of this isseus: (Y axis is a time axis)
[GDBus Worker thread] {Default GLib main context} [Main thread] | | | +-------------------------+ push the idle GSource | | | | <async_init_get_all_cb()> | pop the idle GSource | | receiving GetAll method | to the mainloop | <async_init_get_all_cb()> | | reply message +--------------------------->> | from the mainloop +------------------------------------+ | | +--------------------------->> | | +-------------------------+ | | running <async_init_get_all_cb()> | | | | callback function, and adding the | +-------------------------+ push the idle GSource | | idle GSource user-defined | | | <on_properties_changed()> | push the idle GSource | GDBusProxy ready callback function | | ((unexpected signal!)) | to the mainloop | <on_proxy_ready()> | to the mainloop (such as | | receiving the signal of +--------------------------->> | to the mainloop | <on_proxy_ready()> ...) | | properties change | | <<---------------------------+ | | | | +------------------------------------+ +-------------------------+ | | | | | | | +------------------------------------+ | | pop the idle GSource | | | | <on_properties_changed()> | running <on_properties_changed()> | | | from the mainloop | callback function, but the | | +--------------------------->> | GDBusProxy object has not yet been | | | | initialized, so the signal will be | | | | DISCARDED! | | | +------------------------------------+ | | | | | +------------------------------------+ | | pop the idle GSource | | | | <on_proxy_ready()> | running <on_proxy_ready()> | | | from the mainloop | callback function, and call the | | +--------------------------->> | function => | | | | <async_initable_init_finish()> | | | | to set GDBusProxy object | | | | initialization compiled, but some | | | | cache results of properties was | | | | incorrect! | | | | | | | +------------------------------------+ | | | | | | ... ... ...
I think an easy way to fix this problem is to raising idle GSources priority in whole GetAll reply handler flow, to makesure the on_properties_changed() have been executed after async_init_get_all_cb() and on_proxy_ready(). However this occurs rarely, and in any case the signal of properties changed should not be discarded.
There is proposed patch corrects this. 1. add new API <g_simple_async_result_complete_in_idle_full()> in order to raising idle GSources priority. 2. Change idle GSources priority to HIGH in whole GetAll reply handler flow, to avoid the properties_change signal will be discarded when it's fallowing recevie GetAll reply message.
diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index fc9acbe..6b2a434 100644 --- a/gio/gdbusconnection.c +++ b/gio/gdbusconnection.c @@ -1832,7 +1832,7 @@ send_message_with_reply_deliver (SendMessageData *data, gboolean remove)
data->delivered = TRUE;
- g_simple_async_result_complete_in_idle (data->simple); + g_simple_async_result_complete_in_idle_full (data->simple, G_PRIORITY_HIGH); g_object_unref (data->simple); data->simple = NULL;
diff --git a/gio/gdbusproxy.c b/gio/gdbusproxy.c index 33492b7..c7ace07 100644 --- a/gio/gdbusproxy.c +++ b/gio/gdbusproxy.c @@ -1463,7 +1463,7 @@ async_init_get_all_cb (GDBusConnection *connection, (GDestroyNotify) g_variant_unref); }
- g_simple_async_result_complete_in_idle (data->simple); + g_simple_async_result_complete_in_idle_full (data->simple, G_PRIORITY_HIGH); async_init_data_free (data); }
diff --git a/gio/gsimpleasyncresult.c b/gio/gsimpleasyncresult.c index ee30da2..7e9caf3 100644 --- a/gio/gsimpleasyncresult.c +++ b/gio/gsimpleasyncresult.c @@ -778,19 +778,16 @@ complete_in_idle_cb (gpointer data) }
/** - * g_simple_async_result_complete_in_idle: + * g_simple_async_result_complete_in_idle_full: * @simple: a #GSimpleAsyncResult. * - * Completes an asynchronous function in an idle handler in the - * [thread-default main context][g-main-context-push-thread-default] - * of the thread that @simple was initially created in - * (and re-pushes that context around the invocation of the callback). + * This function is the same as g_simple_async_result_complete_in_idle() except that it lets you specify the priority. * * Calling this function takes a reference to @simple for as long as * is needed to complete the call. */ void -g_simple_async_result_complete_in_idle (GSimpleAsyncResult *simple) +g_simple_async_result_complete_in_idle_full (GSimpleAsyncResult *simple, gint priority) { GSource *source;
@@ -799,13 +796,31 @@ g_simple_async_result_complete_in_idle (GSimpleAsyncResult *simple) g_object_ref (simple);
source = g_idle_source_new (); - g_source_set_priority (source, G_PRIORITY_DEFAULT); + g_source_set_priority (source, priority); g_source_set_callback (source, complete_in_idle_cb, simple, g_object_unref);
g_source_attach (source, simple->context); g_source_unref (source); }
+/** + * g_simple_async_result_complete_in_idle: + * @simple: a #GSimpleAsyncResult. + * + * Completes an asynchronous function in an idle handler in the + * [thread-default main context][g-main-context-push-thread-default] + * of the thread that @simple was initially created in + * (and re-pushes that context around the invocation of the callback). + * + * Calling this function takes a reference to @simple for as long as + * is needed to complete the call. + */ +void +g_simple_async_result_complete_in_idle (GSimpleAsyncResult *simple) +{ + g_simple_async_result_complete_in_idle_full (simple, G_PRIORITY_DEFAULT); +} + typedef struct { GSimpleAsyncResult *simple; GCancellable *cancellable; diff --git a/gio/gsimpleasyncresult.h b/gio/gsimpleasyncresult.h index 94412f4..52b1039 100644 --- a/gio/gsimpleasyncresult.h +++ b/gio/gsimpleasyncresult.h @@ -103,6 +103,8 @@ void g_simple_async_result_set_handle_cancellation (GSimpleAsyncR GLIB_AVAILABLE_IN_ALL void g_simple_async_result_complete (GSimpleAsyncResult *simple); GLIB_AVAILABLE_IN_ALL +void g_simple_async_result_complete_in_idle_full (GSimpleAsyncResult *simple, gint priority); +GLIB_AVAILABLE_IN_ALL void g_simple_async_result_complete_in_idle (GSimpleAsyncResult *simple); GLIB_AVAILABLE_IN_ALL void g_simple_async_result_run_in_thread (GSimpleAsyncResult *simple,
-- Regards,
Brant Li
|
Attachment:
glib_gdbusproxy_race_condition.patch
Description: glib_gdbusproxy_race_condition.patch