[buoh/fix-crashes-and-criticals] Do not try to remove already removed GSource




commit 94a9aadef43adb3163d5f0c474a9801c7f1f49b8
Author: Jan Tojnar <jtojnar gmail com>
Date:   Fri Jan 1 04:06:04 2021 +0100

    Do not try to remove already removed GSource
    
    `buoh_view_comic_update_zoom_cb` always returns false so `g_idle_add`
    will remove it after running. But we still want to remove the previous
    execution if it did not run yet and a new one has been scheduled.
    
    We cannot just use the previous source id for removing the callback
    from the queue, though, since if it has already been removed, the id might
    have been reassigned.
    
    We move the variables tracking the source id to the `BuohViewComic` class
    and clear them in the callback so that the class field only ever contains
    a valid id of an event that has not been yet removed.
    
    I also had to add GMutex guards to `set_image_from_loader_source_id`
    in `buoh_view_comic_set_image_from_loader` as `buoh_view_comic_load_cb` will
    run in `comic_loader` thread, not the UI thread, so it might cause races.
    
    Fixes: https://gitlab.gnome.org/GNOME/buoh/issues/9

 src/buoh-view-comic.c | 46 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 13 deletions(-)
---
diff --git a/src/buoh-view-comic.c b/src/buoh-view-comic.c
index c7aa1f4..fe4451a 100644
--- a/src/buoh-view-comic.c
+++ b/src/buoh-view-comic.c
@@ -44,6 +44,10 @@ struct _BuohViewComic {
         BuohViewZoomMode zoom_mode;
         gdouble          scale;
 
+        guint update_zoom_source_id;
+        GMutex set_image_from_loader_source_id_mutex;
+        guint set_image_from_loader_source_id;
+
         BuohComicLoader *comic_loader;
         GdkPixbufLoader *pixbuf_loader;
 };
@@ -116,6 +120,10 @@ buoh_view_comic_init (BuohViewComic *c_view)
         c_view->comic_loader = buoh_comic_loader_new ();
         c_view->comic_data = g_string_sized_new (DATA_SIZE);
 
+        c_view->update_zoom_source_id = 0;
+        g_mutex_init (&c_view->set_image_from_loader_source_id_mutex);
+        c_view->set_image_from_loader_source_id = 0;
+
         gtk_widget_init_template (GTK_WIDGET (c_view));
 
         g_signal_connect_swapped (G_OBJECT (c_view->comic_loader),
@@ -191,6 +199,8 @@ buoh_view_comic_finalize (GObject *object)
                 g_clear_object (&c_view->pixbuf_loader);
         }
 
+        g_mutex_clear (&c_view->set_image_from_loader_source_id_mutex);
+
         if (G_OBJECT_CLASS (buoh_view_comic_parent_class)->finalize) {
                 (* G_OBJECT_CLASS (buoh_view_comic_parent_class)->finalize) (object);
         }
@@ -366,9 +376,13 @@ buoh_view_comic_update_zoom_cb (BuohViewComic *c_view)
         GdkPixbuf *pixbuf;
         gdouble    new_scale;
 
+        if (!g_source_is_destroyed (g_main_current_source ())) {
+                c_view->update_zoom_source_id = 0;
+        }
+
         pixbuf = buoh_comic_get_pixbuf (c_view->comic);
         if (!pixbuf) {
-                return FALSE;
+                return G_SOURCE_REMOVE;
         }
 
         switch (c_view->zoom_mode) {
@@ -402,21 +416,20 @@ buoh_view_comic_update_zoom_cb (BuohViewComic *c_view)
                 buoh_view_comic_zoom (c_view, new_scale, FALSE);
         }
 
-        return FALSE;
+        return G_SOURCE_REMOVE;
 }
 
 static void
 buoh_view_comic_size_allocate (GtkWidget *widget, GtkAllocation *allocation)
 {
         BuohViewComic *c_view = BUOH_VIEW_COMIC (widget);
-        static gint    id = 0;
 
         if (c_view->comic) {
-                if (id > 0) {
-                        g_source_remove (id);
+                if (c_view->update_zoom_source_id > 0) {
+                        g_source_remove (c_view->update_zoom_source_id);
                 }
-                id = g_idle_add ((GSourceFunc) buoh_view_comic_update_zoom_cb,
-                                 c_view);
+                c_view->update_zoom_source_id = g_idle_add ((GSourceFunc) buoh_view_comic_update_zoom_cb,
+                                                         c_view);
         }
 
         GTK_WIDGET_CLASS (buoh_view_comic_parent_class)->size_allocate (widget, allocation);
@@ -693,12 +706,18 @@ buoh_view_comic_set_image_from_loader (BuohViewComic *c_view)
 {
         GdkPixbuf *pixbuf;
 
+        g_mutex_lock (&c_view->set_image_from_loader_source_id_mutex);
+        if (!g_source_is_destroyed (g_main_current_source ())) {
+                c_view->set_image_from_loader_source_id = 0;
+        }
+        g_mutex_unlock (&c_view->set_image_from_loader_source_id_mutex);
+
         pixbuf = gdk_pixbuf_loader_get_pixbuf (c_view->pixbuf_loader);
         if (pixbuf) {
                 buoh_view_comic_set_image_from_pixbuf (c_view, pixbuf);
         }
 
-        return FALSE;
+        return G_SOURCE_REMOVE;
 }
 
 static void
@@ -706,7 +725,6 @@ buoh_view_comic_load_cb (const gchar   *data,
                          guint          len,
                          BuohViewComic *c_view)
 {
-        static guint id = 0;
         GError *error = NULL;
 
         gdk_pixbuf_loader_write (c_view->pixbuf_loader,
@@ -721,11 +739,13 @@ buoh_view_comic_load_cb (const gchar   *data,
         c_view->comic_data = g_string_append_len (c_view->comic_data,
                                                         data, len);
 
-        if (id > 0) {
-                g_source_remove (id);
+        g_mutex_lock (&c_view->set_image_from_loader_source_id_mutex);
+        if (c_view->set_image_from_loader_source_id > 0) {
+                g_source_remove (c_view->set_image_from_loader_source_id);
         }
-        id = g_idle_add ((GSourceFunc) buoh_view_comic_set_image_from_loader,
-                         (gpointer) c_view);
+        c_view->set_image_from_loader_source_id = g_idle_add ((GSourceFunc) 
buoh_view_comic_set_image_from_loader,
+                                                           (gpointer) c_view);
+        g_mutex_unlock (&c_view->set_image_from_loader_source_id_mutex);
 }
 
 static void


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