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




commit 1a524d716f95a8572713e52d492ea80771527333
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 `gdk_threads_{enter,leave}` 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 | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)
---
diff --git a/src/buoh-view-comic.c b/src/buoh-view-comic.c
index c7aa1f4..427f61b 100644
--- a/src/buoh-view-comic.c
+++ b/src/buoh-view-comic.c
@@ -44,6 +44,9 @@ struct _BuohViewComic {
         BuohViewZoomMode zoom_mode;
         gdouble          scale;
 
+        guint update_zoom_source_id;
+        guint set_image_from_loader_source_id;
+
         BuohComicLoader *comic_loader;
         GdkPixbufLoader *pixbuf_loader;
 };
@@ -116,6 +119,9 @@ 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;
+        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),
@@ -366,9 +372,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 +412,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 +702,16 @@ buoh_view_comic_set_image_from_loader (BuohViewComic *c_view)
 {
         GdkPixbuf *pixbuf;
 
+        if (!g_source_is_destroyed (g_main_current_source ())) {
+                c_view->set_image_from_loader_source_id = 0;
+        }
+
         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 +719,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 +733,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);
+        gdk_threads_enter ();
+        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);
+        gdk_threads_leave ();
 }
 
 static void


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