Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in __execute_function #425

Closed
Quipyowert2 opened this issue Feb 4, 2021 · 1 comment · Fixed by #426 or #429
Closed

Memory leak in __execute_function #425

Quipyowert2 opened this issue Feb 4, 2021 · 1 comment · Fixed by #426 or #429
Labels
type:bug Something's broken!

Comments

@Quipyowert2
Copy link
Contributor

Thanks for reporting your bug here! The following template will help with
giving as much information as possible so that it's easier to diagnose and
fix.

Upfront Information

Please provide the following information by running the command and providing
the output.

  • Fvwm3 version (run: fvwm3 --version)
    ccb7eb4 although fvwm3 --version says fvwm3 1.0.3 (1.0.2-48-g356e327f)
  • Linux distribution or BSD name/version
    openSUSE 15.2 WSL
  • Platform (run: uname -sp)
    Linux x86_64

Expected Behaviour

Ideally, no memory leaks.
What were you trying to do? Please explain the problem.
Check fvwm3 for memory leaks and other issues with Valgrind.

Actual Behaviour

valgrind --fullpath-after=$PWD/ --num-callers=100 --leak-check=full fvwm/fvwm3

Relevant parts of Valgrind output:

==1197== 
==1197== 18 bytes in 1 blocks are definitely lost in loss record 74 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x42B7F2: __handle_bpress_on_root (fvwm/events.c:1661)
==1197==    by 0x42BE82: HandleButtonPress (fvwm/events.c:1847)
==1197==    by 0x43049A: dispatch_event (fvwm/events.c:4146)
==1197==    by 0x43056F: HandleEvents (fvwm/events.c:4194)
==1197==    by 0x40B7EA: main (fvwm/fvwm3.c:2560)
==1197== 
==1197== 23 bytes in 1 blocks are definitely lost in loss record 94 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x44B978: StartupStuff (fvwm/fvwm3.c:1538)
==1197==    by 0x430657: My_XNextEvent (fvwm/events.c:4251)
==1197==    by 0x43053F: HandleEvents (fvwm/events.c:4184)
==1197==    by 0x40B7EA: main (fvwm/fvwm3.c:2560)
==1197== 
==1197== 62 bytes in 1 blocks are definitely lost in loss record 180 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x45B437: CMD_Test (fvwm/conditional.c:2297)
==1197==    by 0x4672CC: __execute_function (fvwm/functions.c:626)
==1197==    by 0x46777C: __run_complex_function_items (fvwm/functions.c:820)
==1197==    by 0x467D47: execute_complex_function (fvwm/functions.c:1015)
==1197==    by 0x467354: __execute_function (fvwm/functions.c:646)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x44B978: StartupStuff (fvwm/fvwm3.c:1538)
==1197==    by 0x430657: My_XNextEvent (fvwm/events.c:4251)
==1197==    by 0x43053F: HandleEvents (fvwm/events.c:4184)
==1197==    by 0x40B7EA: main (fvwm/fvwm3.c:2560)
==1197== 
==1197== 82 bytes in 1 blocks are definitely lost in loss record 233 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x47ECD6: run_command_stream (fvwm/read.c:146)
==1197==    by 0x47F1F8: CMD_PipeRead (fvwm/read.c:387)
==1197==    by 0x4672CC: __execute_function (fvwm/functions.c:626)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x45B437: CMD_Test (fvwm/conditional.c:2297)
==1197==    by 0x4672CC: __execute_function (fvwm/functions.c:626)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x47ECD6: run_command_stream (fvwm/read.c:146)
==1197==    by 0x47EF0D: run_command_file (fvwm/read.c:254)
==1197==    by 0x40BC1E: main (fvwm/fvwm3.c:2507)
==1197== 
==1197== 141 bytes in 2 blocks are definitely lost in loss record 268 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x45B437: CMD_Test (fvwm/conditional.c:2297)
==1197==    by 0x4672CC: __execute_function (fvwm/functions.c:626)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x47ECD6: run_command_stream (fvwm/read.c:146)
==1197==    by 0x47EF0D: run_command_file (fvwm/read.c:254)
==1197==    by 0x40BC1E: main (fvwm/fvwm3.c:2507)
==1197== 
==1197== 150 bytes in 5 blocks are definitely lost in loss record 270 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x47ECD6: run_command_stream (fvwm/read.c:146)
==1197==    by 0x47F1F8: CMD_PipeRead (fvwm/read.c:387)
==1197==    by 0x4672CC: __execute_function (fvwm/functions.c:626)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x47ECD6: run_command_stream (fvwm/read.c:146)
==1197==    by 0x47EF0D: run_command_file (fvwm/read.c:254)
==1197==    by 0x40BC1E: main (fvwm/fvwm3.c:2507)
==1197== 
==1197== 291 bytes in 4 blocks are definitely lost in loss record 336 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x46777C: __run_complex_function_items (fvwm/functions.c:820)
==1197==    by 0x467D47: execute_complex_function (fvwm/functions.c:1015)
==1197==    by 0x467354: __execute_function (fvwm/functions.c:646)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x44B978: StartupStuff (fvwm/fvwm3.c:1538)
==1197==    by 0x430657: My_XNextEvent (fvwm/events.c:4251)
==1197==    by 0x43053F: HandleEvents (fvwm/events.c:4184)
==1197==    by 0x40B7EA: main (fvwm/fvwm3.c:2560)
==1197== 
==1197== 874 bytes in 37 blocks are definitely lost in loss record 383 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x475E44: module_input_execute (fvwm/module_interface.c:730)
==1197==    by 0x475EFF: ExecuteCommandQueue (fvwm/module_interface.c:769)
==1197==    by 0x430A3E: My_XNextEvent (fvwm/events.c:4356)
==1197==    by 0x43053F: HandleEvents (fvwm/events.c:4184)
==1197==    by 0x40B7EA: main (fvwm/fvwm3.c:2560)
==1197== 
==1197== 944 bytes in 28 blocks are definitely lost in loss record 385 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x40B694: SetRCDefaults (fvwm/fvwm3.c:1416)
==1197==    by 0x40B694: main (fvwm/fvwm3.c:2462)
==1197== 
==1197== 3,764 bytes in 122 blocks are definitely lost in loss record 451 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x47ECD6: run_command_stream (fvwm/read.c:146)
==1197==    by 0x47EF0D: run_command_file (fvwm/read.c:254)
==1197==    by 0x47F079: CMD_Read (fvwm/read.c:320)
==1197==    by 0x4672CC: __execute_function (fvwm/functions.c:626)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x40B694: SetRCDefaults (fvwm/fvwm3.c:1416)
==1197==    by 0x40B694: main (fvwm/fvwm3.c:2462)
==1197== 
==1197== 13,908 bytes in 328 blocks are definitely lost in loss record 470 of 477
==1197==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1197==    by 0x4A058D: fxmalloc (libs/safemalloc.c:33)
==1197==    by 0x4741A3: expand_vars (fvwm/expand.c:1373)
==1197==    by 0x46703B: __execute_function (fvwm/functions.c:546)
==1197==    by 0x468457: execute_function (fvwm/functions.c:1280)
==1197==    by 0x47ECD6: run_command_stream (fvwm/read.c:146)
==1197==    by 0x47EF0D: run_command_file (fvwm/read.c:254)
==1197==    by 0x40BC1E: main (fvwm/fvwm3.c:2507)
==1197==
==1197== LEAK SUMMARY:
==1197==    definitely lost: 26,267 bytes in 564 blocks
==1197==    indirectly lost: 2,784 bytes in 101 blocks
==1197==      possibly lost: 0 bytes in 0 blocks
==1197==    still reachable: 449,214 bytes in 11,515 blocks
==1197==         suppressed: 0 bytes in 0 blocks
==1197== Reachable blocks (those to which a pointer was found) are not shown.
==1197== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==1197== 
==1197== Use --track-origins=yes to see where uninitialised values come from
==1197== For lists of detected and suppressed errors, rerun with: -s
==1197== ERROR SUMMARY: 212 errors from 27 contexts (suppressed: 40 from 1)

What should have happened, but didn't?

Enabling logging

fvwm3 has a means of logging what it's doing. Enabling this when
reproducing the issue might help. To do this, either change the means fvwm3
is started by adding -v as in:

fvwm3 -v

or, once fvwm3 has loaded, send SIGUSR2 as in:

pkill -USR2 fvwm3

The resulting logfile can be found in $HOME/.fvwm/fvwm3-output.log

Steps to Reproduce

How can the problem be reproduced?

  1. Compile fvwm3 revision ccb7eb4
  2. valgrind --fullpath-after=$PWD/ --num-callers=100 --leak-check=full fvwm/fvwm3
  3. Left click on fvwm3 desktop
  4. Select Quit from the menu that appears.
  5. Valgrind outputs memory leak details on stdout.

For this, the following is helpful:

  • Reduce the problem to the smallest fvwm configuration example (where
    possible). Start with a blank config file (fvwm3 -f/dev/null) and go from
    there.
    config.txt

The memory leak happens with default config and still happens with fvwm3 -f /dev/null

  • Does the problem also happen with Fvwm2?

Yes, but with libgobject memory leaks instead of expand_vars.

Include your configuration with this issue.

Does Fvwm3 crash?

If fvwm3 crashes then check your system for a corefile. This is platform
dependant, however, check if:

  • corefiles are enabled (ulimit -c)
  • A corefile might be in $HOME or /tmp/.
  • If you're using Linux (with systemd), check coredumpctl list.
    coredumpctl may need installing separately.

If you find a corefile, install gdb and run:

gdb /path/to/fvwm3 /path/to/corefile

If you're using coredumpctl then use:

coredumpctl debug

Then from within the (gdb) prompt, issue:

bt full

... and include the output here.

Extra Information

  • Anything else we should know?

There are also possible fontconfig memory leaks but these aren't the focus of the linked pull request.

  • Feel free to take a screen capture or video and upload to this issue if you
    feel it would help.

  • Attach $HOME/.fvwm/fvwm3-output.log from the step above.
    fvwm3-output.log

@Quipyowert2 Quipyowert2 added the type:bug Something's broken! label Feb 4, 2021
Quipyowert2 added a commit to Quipyowert2/fvwm3 that referenced this issue Feb 4, 2021
When FUNC_DONT_EXPAND_COMMAND isn't set, expand_vars allocates a string
and returns it. This string wasn't being freed by the calling function.

Fixes fvwmorg#425.
ThomasAdam pushed a commit that referenced this issue Feb 4, 2021
When FUNC_DONT_EXPAND_COMMAND isn't set, expand_vars allocates a string
and returns it. This string wasn't being freed by the calling function.

Fixes #425.
@ThomasAdam ThomasAdam reopened this Feb 5, 2021
@ThomasAdam
Copy link
Member

Hi @Quipyowert2

I had a report from a user on IRC who had the latest master branch of fvwm3 segfault. When I got him to git revert 4fec0e8229191f3d0726fcd123333b1b04726ea7 the problem went away.

I don't have a lot of time to check the reasons why. The stacktrace is here:

http://www.linkerror.com/stuff/gdb.txt

Thanks!

Quipyowert2 added a commit to Quipyowert2/fvwm3 that referenced this issue Feb 8, 2021
When a command starting with an asterisk was encountered, ModuleConfig
was called, which calls AddToModList which frees its argument. Thus
expaction shouldn't be freed if the command starts with an asterisk, as
it was already freed in AddToModList.

Fixes fvwmorg#425.
Quipyowert2 added a commit to Quipyowert2/fvwm3 that referenced this issue Feb 8, 2021
When a command starting with an asterisk was encountered, ModuleConfig
was called, which calls AddToModList which frees its argument
sometimes. Then __execute_function tries to free the same pointer
again. This commit fixes this by only freeing rline in AddToModList if
it points at xasprintf'd memory, as freeing the memory from xasprintf
won't invalidate expaction.

Fixes fvwmorg#425.
Quipyowert2 added a commit to Quipyowert2/fvwm3 that referenced this issue Feb 8, 2021
When a command starting with an asterisk was encountered, ModuleConfig
was called, which calls AddToModList which frees its argument
sometimes. Then __execute_function tries to free the same pointer
again. This commit fixes this by only freeing rline in AddToModList if
it points at xasprintf'd memory, as freeing the memory from xasprintf
won't invalidate expaction.

Fixes fvwmorg#425.
Quipyowert2 added a commit to Quipyowert2/fvwm3 that referenced this issue Feb 8, 2021
When a command starting with an asterisk was encountered, ModuleConfig
was called, which calls AddToModList which frees its argument
sometimes. Then __execute_function tries to free the same pointer
again. This commit fixes this by only freeing rline in AddToModList if
it points at xasprintf'd memory, as freeing the memory from xasprintf
won't invalidate expaction.

Fixes fvwmorg#425.
Quipyowert2 added a commit to Quipyowert2/fvwm3 that referenced this issue Feb 8, 2021
When a command starting with an asterisk was encountered, ModuleConfig
was called, which calls AddToModList which frees its argument
sometimes. Then __execute_function tries to free the same pointer
again. This commit fixes this by only freeing rline in AddToModList if
it points at xasprintf'd memory, as freeing the memory from xasprintf
won't invalidate expaction.

Fixes fvwmorg#425.
ThomasAdam pushed a commit that referenced this issue Feb 8, 2021
When a command starting with an asterisk was encountered, ModuleConfig
was called, which calls AddToModList which frees its argument
sometimes. Then __execute_function tries to free the same pointer
again. This commit fixes this by only freeing rline in AddToModList if
it points at xasprintf'd memory, as freeing the memory from xasprintf
won't invalidate expaction.

Fixes #425.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something's broken!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants