Commit b4fa5a75 authored by Milan Crha's avatar Milan Crha

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
parent d43dc879
......@@ -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;
}
}
......
......@@ -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);
}
}
}
......
......@@ -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
......
......@@ -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);
......
......@@ -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);
......
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