[nautilus/wip/oholy/progress-info-race] progress-info: Prevent crashes when job is cancelled



commit 023bca205a8fc34bdfcf7273c61f5dc5b50cf3df
Author: Ondrej Holy <oholy redhat com>
Date:   Thu Feb 13 12:12:18 2020 +0100

    progress-info: Prevent crashes when job is cancelled
    
    In order to prevent deadlock when cancelling operations, new thread
    were added by commit 2db9a295. However, this introduce various races.
    One of them has been fixed by commit 45c7c967. However, I am convinced
    that there is still a race when finalizing. One way to fix it is the
    usage of g_thread_join. But I think that it would be better to avoid
    the deadlock another way. Let's instead remove the problematic thread
    and call g_cancellable_cancel without the lock held.
    
    Fixes: https://gitlab.gnome.org/GNOME/nautilus/issues/124

 src/nautilus-progress-info.c | 46 ++++++++++----------------------------------
 1 file changed, 10 insertions(+), 36 deletions(-)
---
diff --git a/src/nautilus-progress-info.c b/src/nautilus-progress-info.c
index e44d34dcc..521bd71ee 100644
--- a/src/nautilus-progress-info.c
+++ b/src/nautilus-progress-info.c
@@ -48,7 +48,6 @@ struct _NautilusProgressInfo
 
     GCancellable *cancellable;
     guint cancellable_id;
-    GCancellable *details_in_thread_cancellable;
 
     GTimer *progress_timer;
 
@@ -95,8 +94,6 @@ nautilus_progress_info_finalize (GObject *object)
     g_clear_pointer (&info->progress_timer, g_timer_destroy);
     g_cancellable_disconnect (info->cancellable, info->cancellable_id);
     g_object_unref (info->cancellable);
-    g_cancellable_cancel (info->details_in_thread_cancellable);
-    g_clear_object (&info->details_in_thread_cancellable);
     g_clear_object (&info->destination);
 
     if (G_OBJECT_CLASS (nautilus_progress_info_parent_class)->finalize)
@@ -301,38 +298,16 @@ queue_idle (NautilusProgressInfo *info,
     }
 }
 
-static void
-set_details_in_thread (GTask                *task,
-                       NautilusProgressInfo *info,
-                       gpointer              user_data,
-                       GCancellable         *cancellable)
-{
-    if (!g_cancellable_is_cancelled (cancellable))
-    {
-        G_LOCK (progress_info);
-        set_details (info, _("Canceled"));
-        info->cancel_at_idle = TRUE;
-        g_timer_stop (info->progress_timer);
-        queue_idle (info, TRUE);
-        G_UNLOCK (progress_info);
-    }
-}
-
 static void
 on_canceled (GCancellable         *cancellable,
              NautilusProgressInfo *info)
 {
-    GTask *task;
-
-    /* We can't do any lock operaton here, since this is probably the main
-     * thread, so modify the details in another thread. Also it can happens
-     * that we were finalizing the object, so create a new cancellable here
-     * so it can be cancelled in finalize */
-    info->details_in_thread_cancellable = g_cancellable_new ();
-    task = g_task_new (info, info->details_in_thread_cancellable, NULL, NULL);
-    g_task_run_in_thread (task, (GTaskThreadFunc) set_details_in_thread);
-
-    g_object_unref (task);
+    G_LOCK (progress_info);
+    set_details (info, _("Canceled"));
+    info->cancel_at_idle = TRUE;
+    g_timer_stop (info->progress_timer);
+    queue_idle (info, TRUE);
+    G_UNLOCK (progress_info);
 }
 
 static void
@@ -428,12 +403,11 @@ nautilus_progress_info_get_progress (NautilusProgressInfo *info)
 void
 nautilus_progress_info_cancel (NautilusProgressInfo *info)
 {
-    G_LOCK (progress_info);
-
-    g_cancellable_cancel (info->cancellable);
-    g_timer_stop (info->progress_timer);
+    GCancellable *cancellable;
 
-    G_UNLOCK (progress_info);
+    cancellable = nautilus_progress_info_get_cancellable (info);
+    g_cancellable_cancel (cancellable);
+    g_object_unref (cancellable);
 }
 
 GCancellable *


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