derf / feh

a fast and light image viewer
https://feh.finalrewind.org
Other
1.52k stars 159 forks source link

Feh crashes with SIGBUS when rapidly changing the underliying file #700

Open itaranto opened 1 year ago

itaranto commented 1 year ago

Assuming I have an initial image target.jpg and another two images: foo.jpg and bar.jpg, then if do:

feh target.jpg &
while true; do cp -v foo.jpg target.jpg && cp -v bar.jpg target.jpg; done

feh almost instantly crashes with SIGBUS.

If I add some amount of time between copying then the problem dissapears:

while true; do cp -v foo.jpg target.jpg && sleep 0.01 && cp -v bar.jpg target.jpg && sleep 0.01; done

I tested both using i3 and Sway (through XWayland).

This is the backtrace reported by gdb:

#0  decode_mcu_fast (MCU_data=0x5650549f5e98, cinfo=0x7fffbc1c54d0) at /usr/src/debug/libjpeg-turbo/libjpeg-turbo-2.1.5.1/jdhuff.c:708
#1  decode_mcu (cinfo=0x7fffbc1c54d0, MCU_data=0x5650549f5e98) at /usr/src/debug/libjpeg-turbo/libjpeg-turbo-2.1.5.1/jdhuff.c:791
#2  0x00007efcfccb8e1f in decompress_onepass (cinfo=0x7fffbc1c54d0, output_buf=0x5650549f5ff0) at /usr/src/debug/libjpeg-turbo/libjpeg-turbo-2.1.5.1/jdcoefct.c:107
#3  0x00007efcfccbacb2 in process_data_simple_main (cinfo=0x7fffbc1c54d0, output_buf=0x7fffbc1c52d0, out_row_ctr=0x7fffbc1c5254, out_rows_avail=2) at /usr/src/debug/libjpeg-turbo/libjpeg-turbo-2.1.5.1/jdmainct.c:300
#4  0x00007efcfccb6114 in jpeg_read_scanlines (cinfo=cinfo@entry=0x7fffbc1c54d0, scanlines=scanlines@entry=0x7fffbc1c52d0, max_lines=2) at /usr/src/debug/libjpeg-turbo/libjpeg-turbo-2.1.5.1/jdapistd.c:298
#5  0x00007efcfe19d424 in _load (im=0x5650549f2950, load_data=1) at /usr/src/debug/imlib2/imlib2-1.11.0/src/modules/loaders/loader_jpeg.c:147
#6  0x00007efcfdda8844 in __imlib_LoadImageWrapper (l=l@entry=0x5650549dd890, im=im@entry=0x5650549f2950, load_data=<optimized out>) at /usr/src/debug/imlib2/imlib2-1.11.0/src/lib/image.c:421
#7  0x00007efcfddac4c6 in __imlib_LoadImage (file=<optimized out>, ila=0x7fffbc1c58f0) at /usr/src/debug/imlib2/imlib2-1.11.0/src/lib/image.c:627
#8  0x00007efcfdd93ed8 in _imlib_load_image_immediately (err=<synthetic pointer>, file=0x5650549cc640 "target.jpg") at /usr/src/debug/imlib2/imlib2-1.11.0/src/lib/api.c:530
#9  imlib_load_image_with_error_return (file=file@entry=0x5650549cc640 "target.jpg", error_return=error_return@entry=0x7fffbc1c5968) at /usr/src/debug/imlib2/imlib2-1.11.0/src/lib/api.c:580
#10 0x000056505332506f in feh_load_image (im=im@entry=0x7fffbc1c59f0, file=file@entry=0x5650549a16d0) at /usr/src/debug/feh-3.9.1/src/imlib.c:344
#11 0x0000565053325bc5 in feh_reload_image (w=0x5650549dd680, resize=0, force_new=0) at /usr/src/debug/feh-3.9.1/src/imlib.c:519
#12 0x000056505333a589 in feh_event_handle_inotify () at /usr/src/debug/feh-3.9.1/src/winwidget.c:786
#13 0x00005650533307e7 in feh_main_iteration (block=block@entry=1) at /usr/src/debug/feh-3.9.1/src/main.c:228
#14 0x000056505332073a in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/feh-3.9.1/src/main.c:105

Environment: OS: Arch Linux WM: i3 4.22 / Sway 1.8.1

itaranto commented 1 year ago

The same but with png files:

#0  inflate_fast (strm=strm@entry=0x55f8383a90a0, start=start@entry=2401) at /usr/src/debug/zlib/zlib-1.2.13/inffast.c:107
#1  0x00007f9241ab59b9 in inflate (strm=strm@entry=0x55f8383a90a0, flush=flush@entry=2) at /usr/src/debug/zlib/zlib-1.2.13/inflate.c:1067
#2  0x00007f92420e956e in png_zlib_inflate (flush=2, png_ptr=0x55f8383a8f60) at /usr/src/debug/libpng/libpng-1.6.39/pngrutil.c:466
#3  png_process_IDAT_data (png_ptr=0x55f8383a8f60, buffer_length=<optimized out>, buffer=<optimized out>) at /usr/src/debug/libpng/libpng-1.6.39/pngpread.c:671
#4  0x00007f92420f1fc4 in png_process_IDAT_data (buffer_length=32768, buffer=<optimized out>, png_ptr=0x55f8383a8f60) at /usr/src/debug/libpng/libpng-1.6.39/pngpread.c:645
#5  png_push_read_IDAT (png_ptr=0x55f8383a8f60) at /usr/src/debug/libpng/libpng-1.6.39/pngpread.c:608
#6  png_process_some_data (info_ptr=0x55f838144ea0, png_ptr=0x55f8383a8f60) at /usr/src/debug/libpng/libpng-1.6.39/pngpread.c:115
#7  png_process_data (buffer_size=94524581105312, buffer=0x55f8383a8f60 "\001", info_ptr=0x55f838144ea0, png_ptr=0x55f8383a8f60) at /usr/src/debug/libpng/libpng-1.6.39/pngpread.c:46
#8  png_process_data (png_ptr=0x55f8383a8f60, info_ptr=0x55f838144ea0, buffer=buffer@entry=0x7f924233f091 "", buffer_size=buffer_size@entry=32780) at /usr/src/debug/libpng/libpng-1.6.39/pngpread.c:36
#9  0x00007f924234a7cf in _load (im=0x55f8383a8e90, load_data=<optimized out>) at /usr/src/debug/imlib2/imlib2-1.11.0/src/modules/loaders/loader_png.c:554
#10 0x00007f9241f41844 in __imlib_LoadImageWrapper (l=l@entry=0x55f838143880, im=im@entry=0x55f8383a8e90, load_data=<optimized out>) at /usr/src/debug/imlib2/imlib2-1.11.0/src/lib/image.c:421
#11 0x00007f9241f454c6 in __imlib_LoadImage (file=<optimized out>, ila=0x7ffdff8aa2b0) at /usr/src/debug/imlib2/imlib2-1.11.0/src/lib/image.c:627
#12 0x00007f9241f2ced8 in _imlib_load_image_immediately (err=<synthetic pointer>, file=0x55f838132640 "target.png") at /usr/src/debug/imlib2/imlib2-1.11.0/src/lib/api.c:530
#13 imlib_load_image_with_error_return (file=file@entry=0x55f838132640 "target.png", error_return=error_return@entry=0x7ffdff8aa328) at /usr/src/debug/imlib2/imlib2-1.11.0/src/lib/api.c:580
#14 0x000055f8362d606f in feh_load_image (im=im@entry=0x7ffdff8aa3b0, file=file@entry=0x55f8381076d0) at /usr/src/debug/feh-3.9.1/src/imlib.c:344
#15 0x000055f8362d6bc5 in feh_reload_image (w=0x55f838143680, resize=0, force_new=0) at /usr/src/debug/feh-3.9.1/src/imlib.c:519
#16 0x000055f8362eb589 in feh_event_handle_inotify () at /usr/src/debug/feh-3.9.1/src/winwidget.c:786
#17 0x000055f8362e17e7 in feh_main_iteration (block=block@entry=1) at /usr/src/debug/feh-3.9.1/src/main.c:228
#18 0x000055f8362d173a in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/feh-3.9.1/src/main.c:105
avlec commented 1 year ago

I suppose the solution would be locking the file to prevent modification while feh is loading the file with something like fcntl() then releasing the lock when the file has finished loading it. However, it looks like we're passing the filename string off to imlib, see here https://github.com/derf/feh/blob/master/src/imlib.c#L344, which it then loads. So I believe this would be a bug with imlib

N-R-K commented 1 year ago

I'm pretty much 100% certain that the reason for this is because imlib2 likes to mmap the file during loading. That script is changing the file during loading (as shown in the backtrace) and thus causing the SIGBUS. The reason it goes away when you add sleep is because it gives imlib2 enough time to decode the file. See also: Use mmap with care

Unrelated, but I'm curious as to how you managed to discover this issue. Rapidly cp-ing file in an infinite while loop doesn't seem like an everyday use-case.

itaranto commented 1 year ago

Unrelated, but I'm curious as to how you managed to discover this issue. Rapidly cp-ing file in an infinite while loop doesn't seem like an everyday use-case.

I made a Neovim plugin to preview images generated using PlantUML.

What it does is to create the first PNG file using plantuml, open that file with feh and then every time you save the PlantUML source file the PNG will be overwritten and reloaded by feh.

itaranto commented 1 year ago

Oh, and BTW, I didn't reproduced this issue just by using this plugin.

The thing is, this plugin made me uncover a bug in another image viewer (imv), and then I used the same test script on feh just to see how it behaved.