! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Random background color on start
Status:
RESOLVED: INVALID
Product:
Ristretto
Component:
Application

Comments

Description xyzdr4gon333 2017-02-05 13:26:59 CET
TL;DR Seems wrong g_value_unset(&val_bg_color) in rstto_image_viewer_realize should not be there, because the value of that boxed pointer is later to be read.

-------------------------------------


After updating my system an old bug reappeared in Ristretto. When opening ristretto with a file instead of black which I set the background color to, it is blue, or wine red or some other relatively random value. I suspect some uninitialized variable, I don't know why.

This bug seems to be similar to https://bugs.launchpad.net/ubuntu/+source/ristretto/+bug/1219126

So I added some debug output in 

#include <stdio.h>

static void
paint_background (GtkWidget *widget, cairo_t *ctx)
{
    RsttoImageViewer *viewer = RSTTO_IMAGE_VIEWER (widget);
    GdkColor *bg_color = NULL;

    /* Determine if we draw the 'default' background-color,
     * or the fullscreen-background-color.
     */
    if (GDK_WINDOW_STATE_FULLSCREEN & gdk_window_get_state (
                gdk_window_get_toplevel (gtk_widget_get_window (widget))))
    {
        bg_color = viewer->priv->bg_color_fs;
        printf( "Use fullscreen background color: " );
    }

    if (NULL == bg_color)
    {
        bg_color = viewer->priv->bg_color;
        printf( "Use normal background color:" );
    }
    printf( "(%i,%i,%i) at %p\n", bg_color->red, bg_color->blue, bg_color->green, (void*) bg_color );

    /* Paint the background-color */
    /******************************/
    gdk_cairo_set_source_color (ctx, bg_color);
    cairo_paint (ctx);
}

First open: src/ristretto test.png

    Gtk-Message: Failed to load module "gail"
    Gtk-Message: Failed to load module "atk-bridge"
    Use normal background color:(22020,0,0) at 0x56049eaf87c0
    Use normal background color:(22020,56432,0) at 0x56049eaf87c0

Second open

    Gtk-Message: Failed to load module "gail"
    Gtk-Message: Failed to load module "atk-bridge"
    Use normal background color:(22080,0,0) at 0x56407b29b6c0
    Use normal background color:(22080,0,0) at 0x56407b29b6c0

The first one was bluish, the second reddish. I wonder why the channels are 16-bit, and why it changes between the first and second output line or why this function is called multiple times anyway.

Setting bg_color which is read and printed out to zero does not work, it results in a segmentation fault, meaning the pointer viewer->priv->bg_color is invalid / not initialized correctly.

What works is to go to preferences and uncheck and then recheck the background color again. But it is tedious for every opened file.


...

So after some work I tracked down the error to the following problem:

    static void
    rstto_image_viewer_realize(GtkWidget *widget)
    {
        GValue val_bg_color = {0, };
        g_value_init (&val_bg_color, GDK_TYPE_COLOR);
        g_object_get_property (
                G_OBJECT(viewer->priv->settings),
                "bgcolor",
                &val_bg_color);
        viewer->priv->bg_color = g_value_get_boxed (&val_bg_color);
        g_value_unset (&val_bg_color);

g_value_get_boxed makes a copy of the property and returns basically a pointer to it. That pointer is saved into viewer->priv->bg_color, but then the location where that saved pointer points to is basically freed with g_value_unset!

====>

Fix: Delete 
    g_value_unset (&val_bg_color);
    g_value_unset (&val_bg_color_fs);
from the end of rstto_image_viewer_realize

But I've only looked at the Ristretto code for one hour for the first time and I never programmed using GLib, so take this with a grain of salt. That's also why I'm not posting a diff, but for me it solves the problem! So I hope this finds its way into the master.
Comment 1 Igor editbugs 2017-02-07 08:16:34 CET
Hi,
Thanks for reporting the issue, and the efforts to fix it! I will look into it.
Comment 2 Igor editbugs 2017-02-07 16:19:42 CET
In fact, I cannot reproduce the bug - for me, the background is always black after I've set the color in preferences.
Can you check the contents of ~/.config/xfce4/xfconf/xfce-perchannel-xml/ristretto.xml on your system?
Comment 3 xyzdr4gon333 2017-02-07 16:54:09 CET
Use after free is not the most reproducible bug, but I'm sure it is a use-after free, because of the segmentation fault when trying to write to the background color. Try it by hardcoding a background color in paint_background:

    static void
    paint_background (GtkWidget *widget, cairo_t *ctx)
    {
        RsttoImageViewer *viewer = RSTTO_IMAGE_VIEWER (widget);
        GdkColor *bg_color = NULL;

        /* Determine if we draw the 'default' background-color,
         * or the fullscreen-background-color.
         */
        if (GDK_WINDOW_STATE_FULLSCREEN & gdk_window_get_state (
                    gdk_window_get_toplevel (gtk_widget_get_window (widget))))
        {
            bg_color = viewer->priv->bg_color_fs;
        }

        if (NULL == bg_color)
        {
            bg_color = viewer->priv->bg_color;
        }

        /**************** ADDED THESE LINES TO HARDCODE BG ****************/
        bg_color->red   = 0;
        bg_color->green = 0;
        bg_color->blue  = 0;

        /* Paint the background-color */
        /******************************/
        gdk_cairo_set_source_color (ctx, bg_color);
        cairo_paint (ctx);
    }

When I do this I get a segmentation fault. Here is the backtrace from gdb:

    Thread 1 "ristretto" received signal SIGSEGV, Segmentation fault.
    0x00007ffff615b835 in g_type_check_instance_is_fundamentally_a () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
    (gdb) bt
    #0  0x00007ffff615b835 in g_type_check_instance_is_fundamentally_a ()
       from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
    #1  0x00007ffff613baf5 in g_object_unref () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
    #2  0x00007ffff7519e59 in gdk_window_process_all_updates () from /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0
    #3  0x00007ffff7846bf1 in ?? () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
    #4  0x00007ffff74f8d37 in ?? () from /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0
    #5  0x00007ffff5e5d6aa in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
    #6  0x00007ffff5e5da60 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
    #7  0x00007ffff5e5dd82 in g_main_loop_run () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
    #8  0x00007ffff78bf3b7 in gtk_main () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
    #9  0x0000555555563cdf in main (argc=<optimized out>, argv=<optimized out>) at main.c:166

Also maybe you could try running valgrind over Ristretto. E.g. in the above case with the segmentation fault I get roughly 8 errors one of which stems from the above modification:

    ==5491== Invalid write of size 2
    ==5491==    at 0x11C9C1: paint_background (image_viewer.c:910)
    ==5491==    by 0x11C9C1: rstto_image_viewer_paint (image_viewer.c:1531)
    ==5491==    by 0x11C9C1: rstto_image_viewer_expose (image_viewer.c:650)
    ....

But with the >original program< I also see the errors from the point I'm trying to make:

    ==6246== Invalid read of size 2
    ==6246==    at 0x549FB1E: gdk_cairo_set_source_color (in /usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0.2400.31)
    ==6246==    by 0x11C9BA: paint_background (image_viewer.c:914)
    ==6246==    by 0x11C9BA: rstto_image_viewer_paint (image_viewer.c:1531)
    ==6246==    by 0x11C9BA: rstto_image_viewer_expose (image_viewer.c:650)
    ...

If all that can not convince you, maybe different versions of GLib behave differently. I use:

    Linux 4.5.0-1-amd64 #1 SMP Debian 4.5.1-1 (2016-04-14) x86_64 GNU/Linux
    libglib2.0-0:amd64       2.50.2-2          amd64             GLib library of C routines
    ristretto                0.8.1-1           amd64             lightweight picture-viewer for the Xfce desktop envir

(I can't update the kernel, because it won't boot up the GUI anymore)

Ristretto was installed using apt-get install -t sid ristretto. In the same manner I got the source code: apt-get source -t sid ristretto.

Here are my settings:

cat ~/.config/xfce4/xfconf/xfce-perchannel-xml/ristretto.xml

    <?xml version="1.0" encoding="UTF-8"?>

    <channel name="ristretto" version="1.0">
      <property name="window" type="empty">
        <property name="navigationbar" type="empty">
          <property name="position" type="string" value="left"/>
        </property>
        <property name="height" type="uint" value="686"/>
        <property name="width" type="uint" value="820"/>
        <property name="bgcolor-override" type="bool" value="true"/>
        <property name="thumbnails" type="empty">
          <property name="hide-fullscreen" type="bool" value="true"/>
          <property name="show" type="bool" value="false"/>
        </property>
      </property>
      <property name="file" type="empty">
        <property name="current-uri" type="string" value="file:///tmp/test.png"/>
      </property>
      <property name="slideshow" type="empty">
        <property name="timeout" type="uint" value="17"/>
      </property>
    </channel>

I find it weird, that it does not contain the background-color itself, only bgcolor-override=True, but maybe black is the default value and therefore omitted?
Comment 4 Igor editbugs 2017-02-09 11:10:38 CET
(In reply to xyzdragon from comment #3)
>         /**************** ADDED THESE LINES TO HARDCODE BG ****************/
>         bg_color->red   = 0;
>         bg_color->green = 0;
>         bg_color->blue  = 0;
> 
>         /* Paint the background-color */
>         /******************************/
>         gdk_cairo_set_source_color (ctx, bg_color);
>         cairo_paint (ctx);
>     }
> 
> When I do this I get a segmentation fault. Here is the backtrace from gdb:

No segfault here after adding these lines, no matter whether background color is set in ristretto preferences.
Comment 5 xyzdr4gon333 2017-02-09 11:18:24 CET
The segfault issue should be as unreliable as the random background color. If the reassigned variables are not some critical ones the segfault might not happen. Did you check valgrind?
Comment 6 Igor editbugs 2017-02-09 11:21:56 CET
In fact, a segfault does happen if "override background color" is not set in preferences. This is understood as bg_color in paint_background() is set to NULL in this case.

I will continue looking into this having enabled "override background color" option.
Comment 7 Igor editbugs 2017-02-09 11:25:27 CET
(In reply to xyzdragon from comment #3)
> Here are my settings:
> 
> cat ~/.config/xfce4/xfconf/xfce-perchannel-xml/ristretto.xml
> 
>     <?xml version="1.0" encoding="UTF-8"?>
> 
>     <channel name="ristretto" version="1.0">
>       <property name="window" type="empty">
>         <property name="navigationbar" type="empty">
>           <property name="position" type="string" value="left"/>
>         </property>
>         <property name="height" type="uint" value="686"/>
>         <property name="width" type="uint" value="820"/>
>         <property name="bgcolor-override" type="bool" value="true"/>
>         <property name="thumbnails" type="empty">
>           <property name="hide-fullscreen" type="bool" value="true"/>
>           <property name="show" type="bool" value="false"/>
>         </property>
>       </property>
>       <property name="file" type="empty">
>         <property name="current-uri" type="string"
> value="file:///tmp/test.png"/>
>       </property>
>       <property name="slideshow" type="empty">
>         <property name="timeout" type="uint" value="17"/>
>       </property>
>     </channel>
> 
> I find it weird, that it does not contain the background-color itself, only
> bgcolor-override=True, but maybe black is the default value and therefore
> omitted?

I think the problem is here.
bgcolor-override=true without actual bgcolor setting means that you've enabled "override background color" setting but haven't chosen the color by clicking the rectangle near the checkbox. Could you please verify your ristretto settings and  check if setting the color resolves the random color issue?
Comment 8 Igor editbugs 2017-02-09 11:50:11 CET
Actually, after having spent some time on it, I started thinking you were right. I've removed the unset calls by https://git.xfce.org/apps/ristretto/commit/?id=de5c70c4a6e3e8ed62a39f94f2d0eb37aa8cc4c6 - it was an attempt to resolve memory leaks but I think it should be done differently.
Thanks for the report and the investigation!
Comment 9 xyzdr4gon333 2017-09-12 12:41:45 CEST
This has been regressed in 0.8.2-1! Why am I the only one who notices this, it is blatant on first or at at least on third open ...
Comment 10 xyzdr4gon333 2018-01-07 04:04:50 CET
Could you please fix this :(
Comment 11 xyzdr4gon333 2018-07-06 10:30:30 CEST
Pretty please? 

(Installed a new system and refound this again. On my own system I'm using my own fix now for a long time already.)
Comment 12 xyzdr4gon333 2018-07-12 11:15:26 CEST
Created attachment 7827 
Example screenshot showing a (random) wine red background instead of the configured black background
Comment 13 Igor editbugs 2019-04-27 17:50:28 CEST
(In reply to xyzdragon from comment #12)
> Created attachment 7827 
> Example screenshot showing a (random) wine red background instead of the
> configured black background
Hi, are you still seeing this issue? For me, it just isn't happening - neither on 3rd, nor on 20th run.

Could you please post your current ristretto.xml file?
Comment 14 xyzdr4gon333 2019-10-04 14:33:05 CEST
Hi, sorry for the late answer, in the meantime my mail expired and so I was not notified. I lately encountered the same issue on a Jetson Nano running Ubuntu, so I was reminded of this issue and checked up on it and was actually just trying to fork my own version. On ristretto 0.10.0, the background color seems to work now, so I think this can be closed

But there are is another quite obnoxious issue: on the right side of the screen is a weird gradient for some reason maybe 20px wide.
Comment 15 xyzdr4gon333 2019-10-04 14:36:10 CEST
Created attachment 9084 
Weird gradient on right (offtopic)
Comment 16 Igor editbugs 2019-10-04 14:39:48 CEST
Hi, thanks for the update!

Yes, the gradient is (supposedly) an artifact of GtkScrolledWindow. I may have some ideas on how it can be fixed and will look into it.
Comment 17 xyzdr4gon333 2019-10-04 15:09:41 CEST
Thanks for the hint. Creating a file ~/.config/gtk-3.0/gtk.css with the contents:

placessidebar overshoot.top, scrolledwindow overshoot.top { background-image: none; }
placessidebar overshoot.top:backdrop, scrolledwindow overshoot.top:backdrop { background-image: none; }
placessidebar overshoot.bottom, scrolledwindow overshoot.bottom { background-image: none; }
placessidebar overshoot.bottom:backdrop, scrolledwindow overshoot.bottom:backdrop { background-image: none; }
placessidebar overshoot.left, scrolledwindow overshoot.left { background-image: none; }
placessidebar overshoot.left:backdrop, scrolledwindow overshoot.left:backdrop { background-image: none; }
placessidebar overshoot.right, scrolledwindow overshoot.right { background-image: none; }
placessidebar overshoot.right:backdrop, scrolledwindow overshoot.right:backdrop { background-image: none; }
placessidebar undershoot.top, scrolledwindow undershoot.top { background-image: none; }
placessidebar undershoot.bottom, scrolledwindow undershoot.bottom { background-image: none; }
placessidebar undershoot.left, scrolledwindow undershoot.left { background-image: none; }
placessidebar undershoot.right, scrolledwindow undershoot.right { background-image: none; }

Disables the gradients, which I don't need anyways. However, they should appear when there is no scrollbar necessary in the first place. When scrollbars are there, it might make sense.
Comment 18 xyzdr4gon333 2019-10-04 15:10:28 CEST
Edit: they should NOT appear when there is no scrollbar necessary

Bug #13335

Reported by:
xyzdr4gon333
Reported on: 2017-02-05
Last modified on: 2019-10-04

People

CC List:
2 users

Version

Attachments

Additional information