[gvfs/gnome-3-32] gmountsource: Fix deadlocks in synchronous API



commit e5c4981478fe26800b0c8362fd4d15d5c8bc0279
Author: Ondrej Holy <oholy redhat com>
Date:   Tue Apr 2 16:36:16 2019 +0200

    gmountsource: Fix deadlocks in synchronous API
    
    Synchronous GMountSource API is implemented using the asynchronous API
    with help of mutexes and conditions. But it seems that it is not guaranteed
    that GAsyncReadyCallback is called on another thread and thus this solution
    may cause deadlocks consequently. Let's rather use solution based on custom
    mainloop to prevent potential deadlocks. Alternatively, we could use
    synchronous API generated by gdbus-codegen to implement this functionality,
    but the solution with custom mainloop seems to be less error-prone.
    
    Closes: https://gitlab.gnome.org/GNOME/gvfs/issues/383

 common/gmountsource.c | 68 ++++++++++++++++++++++-----------------------------
 1 file changed, 29 insertions(+), 39 deletions(-)
---
diff --git a/common/gmountsource.c b/common/gmountsource.c
index c5d9f1a3..d3307642 100644
--- a/common/gmountsource.c
+++ b/common/gmountsource.c
@@ -146,8 +146,8 @@ typedef struct AskSyncData AskSyncData;
 struct AskSyncData {
 
   /* For sync calls */
-  GMutex mutex;
-  GCond cond;
+  GMainContext *context;
+  GMainLoop *loop;
 
   /* results: */
   GAsyncResult *result;
@@ -375,10 +375,7 @@ ask_reply_sync  (GObject *source_object,
 
   data->result = g_object_ref (res);
 
-  /* Wake up sync call thread */
-  g_mutex_lock (&data->mutex);
-  g_cond_signal (&data->cond);
-  g_mutex_unlock (&data->mutex);
+  g_main_loop_quit (data->loop);
 }
 
 gboolean
@@ -397,11 +394,10 @@ g_mount_source_ask_password (GMountSource *source,
   gboolean handled;
   AskSyncData data;
 
-  memset (&data, 0, sizeof (data));
-  g_mutex_init (&data.mutex);
-  g_cond_init (&data.cond);
-  g_mutex_lock (&data.mutex);
+  data.context = g_main_context_new ();
+  data.loop = g_main_loop_new (data.context, FALSE);
 
+  g_main_context_push_thread_default (data.context);
 
   g_mount_source_ask_password_async (source,
                                      message_string,
@@ -411,13 +407,7 @@ g_mount_source_ask_password (GMountSource *source,
                                      ask_reply_sync,
                                      &data);
 
-  while (data.result == NULL)
-    g_cond_wait (&data.cond, &data.mutex);
-  g_mutex_unlock (&data.mutex);
-
-  g_cond_clear (&data.cond);
-  g_mutex_clear (&data.mutex);
-
+  g_main_loop_run (data.loop);
 
   handled = g_mount_source_ask_password_finish (source,
                                                 data.result,
@@ -427,6 +417,10 @@ g_mount_source_ask_password (GMountSource *source,
                                                 domain_out,
                                                anonymous_out,
                                                password_save_out);
+
+  g_main_context_pop_thread_default (data.context);
+  g_main_context_unref (data.context);
+  g_main_loop_unref (data.loop);
   g_object_unref (data.result);
 
   return handled;
@@ -562,11 +556,11 @@ g_mount_source_ask_question (GMountSource *source,
   gint choice;
   gboolean handled, aborted;
   AskSyncData data;
-  
-  memset (&data, 0, sizeof (data));
-  g_mutex_init (&data.mutex);
-  g_cond_init (&data.cond);
-  g_mutex_lock (&data.mutex);
+
+  data.context = g_main_context_new ();
+  data.loop = g_main_loop_new (data.context, FALSE);
+
+  g_main_context_push_thread_default (data.context);
 
   g_mount_source_ask_question_async (source,
                                      message,
@@ -574,18 +568,16 @@ g_mount_source_ask_question (GMountSource *source,
                                      ask_reply_sync,
                                      &data);
 
-  while (data.result == NULL)
-    g_cond_wait (&data.cond, &data.mutex);
-  g_mutex_unlock (&data.mutex);
-
-  g_cond_clear (&data.cond);
-  g_mutex_clear (&data.mutex);
+  g_main_loop_run (data.loop);
 
   handled = g_mount_source_ask_question_finish (source,
                                                 data.result,
                                                 &aborted,
                                                 &choice);
-  
+
+  g_main_context_pop_thread_default (data.context);
+  g_main_context_unref (data.context);
+  g_main_loop_unref (data.loop);
   g_object_unref (data.result);
 
   if (aborted_out)
@@ -833,10 +825,10 @@ g_mount_source_show_processes (GMountSource *source,
   gboolean handled, aborted;
   AskSyncData data;
 
-  memset (&data, 0, sizeof (data));
-  g_mutex_init (&data.mutex);
-  g_cond_init (&data.cond);
-  g_mutex_lock (&data.mutex);
+  data.context = g_main_context_new ();
+  data.loop = g_main_loop_new (data.context, FALSE);
+
+  g_main_context_push_thread_default (data.context);
 
   g_mount_source_show_processes_async (source,
                                        message,
@@ -845,18 +837,16 @@ g_mount_source_show_processes (GMountSource *source,
                                        ask_reply_sync,
                                        &data);
 
-  while (data.result == NULL)
-    g_cond_wait (&data.cond, &data.mutex);
-  g_mutex_unlock (&data.mutex);
-
-  g_cond_clear (&data.cond);
-  g_mutex_clear (&data.mutex);
+  g_main_loop_run (data.loop);
 
   handled = g_mount_source_show_processes_finish (source,
                                                   data.result,
                                                   &aborted,
                                                   &choice);
 
+  g_main_context_pop_thread_default (data.context);
+  g_main_context_unref (data.context);
+  g_main_loop_unref (data.loop);
   g_object_unref (data.result);
 
   if (aborted_out)


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