memory leak when using --reload on http images

[derf]

feh --reload 1 http://... leaks about 4 kilobytes per 15 seconds. It may be related to the filelist handling in cb_reload_timer, but I'm not entirely sure yet.

[livibetter]

I think the issue might be in feh_reload_image(), I checked out one revision before I submitted my reload commit and it still leaks. (I was afraid it's my fault).

So, I commented out feh_reload_image() in the reload timer, it stops leaking, but I can't spot leaking code with my eyes.

I am installing valgrind, not sure I know how to use it...

[livibetter]

==3881== Memcheck, a memory error detector
==3881== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==3881== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==3881== Command: feh --reload 1 https://a248.e.akamai.net/assets.github.com/images/modules/header/logov6-hover.png
==3881==
==3881==
==3881== HEAP SUMMARY:
==3881==     in use at exit: 193,868 bytes in 2,264 blocks
==3881==   total heap usage: 12,566,671 allocs, 12,564,407 frees, 610,099,246 bytes allocated
==3881==
==3881== 17 bytes in 1 blocks are definitely lost in loss record 46 of 205
==3881==    at 0x4C267CE: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3881==    by 0x41A5F5: _estrdup (utils.c:76)
==3881==    by 0x408203: feh_file_new (filelist.c:49)
==3881==    by 0x4084E7: add_file_to_filelist_recursively (filelist.c:146)
==3881==    by 0x41467B: feh_parse_option_array (options.c:795)
==3881==    by 0x414D63: init_parse_options (options.c:94)
==3881==    by 0x40F79E: main (main.c:43)
==3881==
==3881== 44 (40 direct, 4 indirect) bytes in 1 blocks are definitely lost in loss record 85 of 205
==3881==    at 0x4C267CE: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3881==    by 0x41A5B8: _emalloc (utils.c:88)
==3881==    by 0x407C3D: feh_file_info_new (filelist.c:77)
==3881==    by 0x407D05: feh_file_info_load (filelist.c:330)
==3881==    by 0x4094DE: feh_load_image (imlib.c:147)
==3881==    by 0x41CDA9: winwidget_create_from_file (winwidget.c:737)
==3881==    by 0x4160FB: init_slideshow_mode (slideshow.c:62)
==3881==    by 0x40F87B: main (main.c:75)
==3881==
==3881== 83 bytes in 1 blocks are definitely lost in loss record 121 of 205
==3881==    at 0x4C267CE: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3881==    by 0x41A5F5: _estrdup (utils.c:76)
==3881==    by 0x4081E5: feh_file_new (filelist.c:46)
==3881==    by 0x4084E7: add_file_to_filelist_recursively (filelist.c:146)
==3881==    by 0x41467B: feh_parse_option_array (options.c:795)
==3881==    by 0x414D63: init_parse_options (options.c:94)
==3881==    by 0x40F79E: main (main.c:43)
==3881==
==3881== 124 bytes in 1 blocks are definitely lost in loss record 129 of 205
==3881==    at 0x4C267CE: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3881==    by 0x73DD079: ??? (in /usr/lib64/libxcb.so.1.1.0)
==3881==    by 0x73DD145: ??? (in /usr/lib64/libxcb.so.1.1.0)
==3881==    by 0x73DCC78: xcb_connect_to_display_with_auth_info (in /usr/lib64/libxcb.so.1.1.0)
==3881==    by 0x57749C9: _XConnectXCB (in /usr/lib64/libX11.so.6.3.0)
==3881==    by 0x57645D6: XOpenDisplay (in /usr/lib64/libX11.so.6.3.0)
==3881==    by 0x409A67: init_x_and_imlib (imlib.c:84)
==3881==    by 0x40F814: main (main.c:48)
==3881==
==3881== 374 bytes in 22 blocks are definitely lost in loss record 155 of 205
==3881==    at 0x4C267CE: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3881==    by 0x41A5F5: _estrdup (utils.c:76)
==3881==    by 0x408203: feh_file_new (filelist.c:49)
==3881==    by 0x4084E7: add_file_to_filelist_recursively (filelist.c:146)
==3881==    by 0x415EF9: cb_reload_timer (slideshow.c:107)
==3881==    by 0x41A27E: feh_handle_timer (timers.c:44)
==3881==    by 0x40F6B4: feh_main_iteration (main.c:168)
==3881==    by 0x40F7F1: main (main.c:79)
==3881==
==3881== 968 (880 direct, 88 indirect) bytes in 22 blocks are definitely lost in loss record 178 of 205
==3881==    at 0x4C267CE: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3881==    by 0x41A5B8: _emalloc (utils.c:88)
==3881==    by 0x407C3D: feh_file_info_new (filelist.c:77)
==3881==    by 0x407D05: feh_file_info_load (filelist.c:330)
==3881==    by 0x4094DE: feh_load_image (imlib.c:147)
==3881==    by 0x415D15: feh_reload_image (slideshow.c:172)
==3881==    by 0x415FB3: cb_reload_timer (slideshow.c:138)
==3881==    by 0x41A27E: feh_handle_timer (timers.c:44)
==3881==    by 0x40F6B4: feh_main_iteration (main.c:168)
==3881==    by 0x40F7F1: main (main.c:79)
==3881==
==3881== 1,826 bytes in 22 blocks are definitely lost in loss record 185 of 205
==3881==    at 0x4C267CE: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3881==    by 0x41A5F5: _estrdup (utils.c:76)
==3881==    by 0x4081E5: feh_file_new (filelist.c:46)
==3881==    by 0x4084E7: add_file_to_filelist_recursively (filelist.c:146)
==3881==    by 0x415EF9: cb_reload_timer (slideshow.c:107)
==3881==    by 0x41A27E: feh_handle_timer (timers.c:44)
==3881==    by 0x40F6B4: feh_main_iteration (main.c:168)
==3881==    by 0x40F7F1: main (main.c:79)
==3881==
==3881== LEAK SUMMARY:
==3881==    definitely lost: 3,344 bytes in 70 blocks
==3881==    indirectly lost: 92 bytes in 23 blocks
==3881==      possibly lost: 0 bytes in 0 blocks
==3881==    still reachable: 190,432 bytes in 2,171 blocks
==3881==         suppressed: 0 bytes in 0 blocks
==3881== Reachable blocks (those to which a pointer was found) are not shown.
==3881== To see them, rerun with: --leak-check=full --show-reachable=yes
==3881==
==3881== For counts of detected and suppressed errors, rerun with: -v
==3881== ERROR SUMMARY: 7 errors from 7 contexts (suppressed: 57 from 9)

[livibetter]

I commented out my addition for directory reload to reduce the output of valgrind and one hunted down:

diff --git a/src/filelist.c b/src/filelist.c
index 215f371..81d84c2 100644
--- a/src/filelist.c
+++ b/src/filelist.c
@@ -327,6 +327,8 @@ int feh_file_info_load(feh_file * file, Imlib_Image im)
        if (!im1)
                return(1);

+       if (file->info)
+               feh_file_info_free(file->info);
        file->info = feh_file_info_new();

        file->info->width = gib_imlib_image_get_width(im1);

[livibetter]

Never mind my previous one, it doesn't matter if

diff --git a/src/slideshow.c b/src/slideshow.c
index 22d2124..c378c2c 100644
--- a/src/slideshow.c
+++ b/src/slideshow.c
@@ -96,6 +96,10 @@ void cb_reload_timer(void *data)

        /* save the current filename for refinding it in new list */
        current_filename = estrdup(FEH_FILE(current_file->data)->filename);
+       for (l = filelist; l; l = l->next) {
+               feh_file_free(l->data);
+               l->data = NULL;
+               }
        gib_list_free_and_data(filelist);
        filelist = NULL;
        filelist_len = 0;

(It's my fault.)

It's all clean except the last one:

==4820== 124 bytes in 1 blocks are definitely lost in loss record 124 of 196
==4820==    at 0x4C267CE: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==4820==    by 0x73DD079: ??? (in /usr/lib64/libxcb.so.1.1.0)
==4820==    by 0x73DD145: ??? (in /usr/lib64/libxcb.so.1.1.0)
==4820==    by 0x73DCC78: xcb_connect_to_display_with_auth_info (in /usr/lib64/libxcb.so.1.1.0)
==4820==    by 0x57749C9: _XConnectXCB (in /usr/lib64/libX11.so.6.3.0)
==4820==    by 0x57645D6: XOpenDisplay (in /usr/lib64/libX11.so.6.3.0)
==4820==    by 0x409A67: init_x_and_imlib (imlib.c:84)
==4820==    by 0x40F814: main (main.c:48)

Edit:

It's xcb and it's been fixed but doesn't seem to be released, my xcb is 1.7.

[derf]

Great, thanks!

I also have libxcb 1.7 with that memleak here, but yeah, we can only wait 'til they release a version including the fix.