[evolution] ESimpleAsyncResult: Avoid thread scheduling race when finishing on idle

[Milan Crha <mcrha src gnome org>]

commit b4fa5a751af94faba57461fd767d5d3517b22d4c
Author: Milan Crha <mcrha redhat com>
Date:   Mon Nov 11 18:09:57 2019 +0100

    ESimpleAsyncResult: Avoid thread scheduling race when finishing on idle
    
    The e_simple_async_result_complete_idle() adds a reference to the passed-in
    ESimpleAsyncResult instance and schedules an idle callback, where it is
    also unreffed. Usual follow up action of the caller was to unref the result,
    maybe in a dedicated thread. The race comes when the dedicated thread is
    suspended before it removes its reference on the result and the idle callback
    is processed before the thread is resumed. That can also mean to free
    widgets, which are supposed to be freed only in the main/UI thread,
    in the dedicated thread, causing crash or other issues. The fix is to
    take the reference of the result, instead of adding its own and unreffing
    it shortly after.
    
    This is related to a downstream bug report:
    https://bugzilla.redhat.com/show_bug.cgi?id=1770923

 src/e-util/e-collection-account-wizard.c | 12 ++++++------
 src/e-util/e-config-lookup.c             |  6 ++----
 src/e-util/e-simple-async-result.c       | 15 ++++++++++++---
 src/e-util/e-simple-async-result.h       |  2 ++
 src/libemail-engine/mail-folder-cache.c  |  6 +++---
 5 files changed, 25 insertions(+), 16 deletions(-)
---
diff --git a/src/e-util/e-collection-account-wizard.c b/src/e-util/e-collection-account-wizard.c
index 158198d9b5..62a1790f7c 100644
--- a/src/e-util/e-collection-account-wizard.c
+++ b/src/e-util/e-collection-account-wizard.c
@@ -538,8 +538,8 @@ collection_account_wizard_worker_finished_cb (EConfigLookup *config_lookup,
                }
 
                if (wizard->priv->running_result) {
-                       e_simple_async_result_complete_idle (wizard->priv->running_result);
-                       g_clear_object (&wizard->priv->running_result);
+                       e_simple_async_result_complete_idle_take (wizard->priv->running_result);
+                       wizard->priv->running_result = NULL;
                }
 
                g_object_notify (G_OBJECT (wizard), "can-run");
@@ -2111,8 +2111,8 @@ collection_account_wizard_dispose (GObject *object)
        }
 
        if (wizard->priv->running_result) {
-               e_simple_async_result_complete_idle (wizard->priv->running_result);
-               g_clear_object (&wizard->priv->running_result);
+               e_simple_async_result_complete_idle_take (wizard->priv->running_result);
+               wizard->priv->running_result = NULL;
        }
 
        for (ii = 0; ii <= E_CONFIG_LOOKUP_RESULT_LAST_KIND; ii++) {
@@ -2611,8 +2611,8 @@ e_collection_account_wizard_run (ECollectionAccountWizard *wizard,
        }
 
        if (!any_worker) {
-               e_simple_async_result_complete_idle (wizard->priv->running_result);
-               g_clear_object (&wizard->priv->running_result);
+               e_simple_async_result_complete_idle_take (wizard->priv->running_result);
+               wizard->priv->running_result = NULL;
        }
 }
 
diff --git a/src/e-util/e-config-lookup.c b/src/e-util/e-config-lookup.c
index 3b68fa6876..a16225f9bf 100644
--- a/src/e-util/e-config-lookup.c
+++ b/src/e-util/e-config-lookup.c
@@ -196,8 +196,7 @@ config_lookup_thread (gpointer data,
        g_mutex_unlock (&config_lookup->priv->property_lock);
 
        if (run_result) {
-               e_simple_async_result_complete_idle (run_result);
-               g_object_unref (run_result);
+               e_simple_async_result_complete_idle_take (run_result);
        }
 
        e_named_parameters_free (restart_params);
@@ -751,8 +750,7 @@ e_config_lookup_run (EConfigLookup *config_lookup,
                g_mutex_unlock (&config_lookup->priv->property_lock);
 
                if (run_result) {
-                       e_simple_async_result_complete_idle (run_result);
-                       g_object_unref (run_result);
+                       e_simple_async_result_complete_idle_take (run_result);
                }
        }
 }
diff --git a/src/e-util/e-simple-async-result.c b/src/e-util/e-simple-async-result.c
index 81f50d6806..ba3159f33c 100644
--- a/src/e-util/e-simple-async-result.c
+++ b/src/e-util/e-simple-async-result.c
@@ -273,9 +273,8 @@ e_simple_async_result_thread (gpointer data,
                g_async_result_get_source_object (G_ASYNC_RESULT (td->result)),
                td->cancellable);
 
-       e_simple_async_result_complete_idle (td->result);
+       e_simple_async_result_complete_idle_take (td->result);
 
-       g_clear_object (&td->result);
        g_clear_object (&td->cancellable);
        g_free (td);
 }
@@ -346,7 +345,17 @@ e_simple_async_result_complete_idle (ESimpleAsyncResult *result)
 {
        g_return_if_fail (E_IS_SIMPLE_ASYNC_RESULT (result));
 
-       g_idle_add (result_complete_idle_cb, g_object_ref (result));
+       e_simple_async_result_complete_idle_take (g_object_ref (result));
+}
+
+/* The same as e_simple_async_result_complete_idle(), but assumes ownership
+   of the 'result' argument. */
+void
+e_simple_async_result_complete_idle_take (ESimpleAsyncResult *result)
+{
+       g_return_if_fail (E_IS_SIMPLE_ASYNC_RESULT (result));
+
+       g_idle_add (result_complete_idle_cb, result);
 }
 
 void
diff --git a/src/e-util/e-simple-async-result.h b/src/e-util/e-simple-async-result.h
index a53a6cec74..e065732f89 100644
--- a/src/e-util/e-simple-async-result.h
+++ b/src/e-util/e-simple-async-result.h
@@ -99,6 +99,8 @@ void          e_simple_async_result_run_in_thread
 void           e_simple_async_result_complete  (ESimpleAsyncResult *result);
 void           e_simple_async_result_complete_idle
                                                (ESimpleAsyncResult *result);
+void           e_simple_async_result_complete_idle_take
+                                               (ESimpleAsyncResult *result);
 void           e_simple_async_result_take_error
                                                (ESimpleAsyncResult *result,
                                                 GError *error);
diff --git a/src/libemail-engine/mail-folder-cache.c b/src/libemail-engine/mail-folder-cache.c
index d86e544202..653164537c 100644
--- a/src/libemail-engine/mail-folder-cache.c
+++ b/src/libemail-engine/mail-folder-cache.c
@@ -2229,9 +2229,9 @@ exit:
                 * e_simple_async_result_run_in_thread() will complete it
                 * for us, and we don't want to complete it twice. */
                if (queued_result != simple)
-                       e_simple_async_result_complete_idle (queued_result);
-
-               g_clear_object (&queued_result);
+                       e_simple_async_result_complete_idle_take (queued_result);
+               else
+                       g_clear_object (&queued_result);
        }
 
        g_object_unref (session);