glib2.40.0 Race condition during initialization of GDBusProxy object.



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



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]