herrhotzenplotz / gcli

Portable Git(hub|lab|tea) CLI tool
https://herrhotzenplotz.de/gcli/
BSD 2-Clause "Simplified" License
52 stars 2 forks source link

Valgrind is reporting multiple memory leaks #122

Open MikaelFangel opened 11 months ago

MikaelFangel commented 11 months ago

I've run a memory check with valgrind on the application, and it looks like the application has several memory leaks.

Simple example

valgrind --leak-check=yes ./result/bin/gcli
==190672== Memcheck, a memory error detector
==190672== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==190672== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==190672== Command: ./result/bin/gcli
==190672== 
error: missing subcommand
usage: gcli [options] subcommand

OPTIONS:
  -a account     Use the configured account instead of inferring it
  -r remote      Infer account from the given git remote
  -t type        Force the account type:
                    - github (default: github.com)
                    - gitlab (default: gitlab.com)
                    - gitea (default: codeberg.org)
  -c             Force colour and text formatting.
  -q             Be quiet. (Not implemented yet)

  -v             Be verbose.

SUBCOMMANDS:
  ci             Github CI status info
  comment        Comment under issues and PRs
  config         Configure forges
  forks          Create, delete and list repository forks
  gists          Create, fetch and list Github Gists
  issues         Manage issues
  labels         Manage issue and PR labels
  milestones     Milestone handling
  pipelines      Gitlab CI management
  pulls          Create, view and manage PRs
  releases       Manage releases of repositories
  repos          Remote Repository management
  snippets       Fetch and list Gitlab snippets
  status         General user status and notifications
  api            Fetch plain JSON info from an API (for debugging purposes)
  version        Print version

gcli 2.0.0 (x86_64-pc-linux-gnu)
Using libcurl/8.4.0 OpenSSL/3.0.12 zlib/1.3 brotli/1.1.0 zstd/1.5.5 libidn2/2.3.4 libssh2/1.11.0 nghttp2/1.57.0
Using vendored pdjson library
Report bugs at https://gitlab.com/herrhotzenplotz/gcli/.
Copyright 2021, 2022, 2023 Nico Sonack <nsonack@herrhotzenplotz.de> and contributors.
==190672== 
==190672== HEAP SUMMARY:
==190672==     in use at exit: 176 bytes in 2 blocks
==190672==   total heap usage: 2 allocs, 0 frees, 176 bytes allocated
==190672== 
==190672== LEAK SUMMARY:
==190672==    definitely lost: 0 bytes in 0 blocks
==190672==    indirectly lost: 0 bytes in 0 blocks
==190672==      possibly lost: 0 bytes in 0 blocks
==190672==    still reachable: 176 bytes in 2 blocks
==190672==         suppressed: 0 bytes in 0 blocks
==190672== Reachable blocks (those to which a pointer was found) are not shown.
==190672== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==190672== 
==190672== For lists of detected and suppressed errors, rerun with: -s

Larger example

valgrind --leak-check=yes ./result/bin/gcli -t github ci -o mikaelfangel -r nixvim-config -n 5 main
==186213== Memcheck, a memory error detector
==186213== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==186213== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==186213== Command: ./result/bin/gcli -t github ci -o mikaelfangel -r nixvim-config -n 5 main
==186213== 
ID           STATUS     CONCLUSION  STARTED               COMPLETED             NAME
19025257350  completed  success     2023-11-26T02:54:00Z  2023-11-26T02:56:18Z  tests (mikaelfangel, nixpkgs=channel:nixpkgs-unstable)
19025257314  completed  success     2023-11-26T02:53:58Z  2023-11-26T02:56:12Z  tests (mikaelfangel, nixpkgs=channel:nixos-unstable)
19012259503  completed  success     2023-11-25T02:54:11Z  2023-11-25T02:56:18Z  tests (mikaelfangel, nixpkgs=channel:nixpkgs-unstable)
19012259442  completed  success     2023-11-25T02:54:12Z  2023-11-25T02:56:18Z  tests (mikaelfangel, nixpkgs=channel:nixos-unstable)
18984627698  completed  success     2023-11-24T02:54:15Z  2023-11-24T02:56:22Z  tests (mikaelfangel, nixpkgs=channel:nixpkgs-unstable)
18984627607  completed  success     2023-11-24T02:54:14Z  2023-11-24T02:56:23Z  tests (mikaelfangel, nixpkgs=channel:nixos-unstable)
18958646237  completed  success     2023-11-23T08:24:51Z  2023-11-23T08:26:53Z  tests (mikaelfangel, nixpkgs=channel:nixpkgs-unstable)
18958646039  completed  success     2023-11-23T08:24:52Z  2023-11-23T08:25:07Z  format
18958646038  completed  success     2023-11-23T08:24:49Z  2023-11-23T08:26:51Z  tests (mikaelfangel, nixpkgs=channel:nixos-unstable)
==186213== 
==186213== HEAP SUMMARY:
==186213==     in use at exit: 986,773 bytes in 14,496 blocks
==186213==   total heap usage: 301,882 allocs, 287,386 frees, 12,808,923 bytes allocated
==186213== 
==186213== 23 bytes in 1 blocks are definitely lost in loss record 36 of 356
==186213==    at 0x484276B: malloc (in /nix/store/4picxabc7pfzzxk3myzckpifn8dnpvcb-valgrind-3.22.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==186213==    by 0x49E341D: strdup (in /nix/store/qn3ggz5sf3hkjs2c797xf7nan3amdxmp-glibc-2.38-27/lib/libc.so.6)
==186213==    by 0x486B5A5: github_get_checks (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/lib/libgcli.so.0.0.0)
==186213==    by 0x404FA1: github_checks (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x405223: subcommand_ci (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x404C2C: main (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213== 
==186213== 33 bytes in 1 blocks are definitely lost in loss record 98 of 356
==186213==    at 0x4849C34: calloc (in /nix/store/4picxabc7pfzzxk3myzckpifn8dnpvcb-valgrind-3.22.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==186213==    by 0x48780AA: sn_asprintf (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/lib/libgcli.so.0.0.0)
==186213==    by 0x405B0B: ensure_config (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x405F6E: gcli_config_find_by_key (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x40656B: gcli_config_get_account (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x406608: gcli_config_get_apibase (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x486B5A5: github_get_checks (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/lib/libgcli.so.0.0.0)
==186213==    by 0x404FA1: github_checks (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x405223: subcommand_ci (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x404C2C: main (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213== 
==186213== LEAK SUMMARY:
==186213==    definitely lost: 56 bytes in 2 blocks
==186213==    indirectly lost: 0 bytes in 0 blocks
==186213==      possibly lost: 0 bytes in 0 blocks
==186213==    still reachable: 986,717 bytes in 14,494 blocks
==186213==         suppressed: 0 bytes in 0 blocks
==186213== Reachable blocks (those to which a pointer was found) are not shown.
==186213== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==186213== 
==186213== For lists of detected and suppressed errors, rerun with: -s
==186213== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
herrhotzenplotz commented 11 months ago

I've run a memory check with valgrind on the application, and it looks like the application has several memory leaks.

==190672== HEAP SUMMARY:
==190672==     in use at exit: 176 bytes in 2 blocks
==190672==   total heap usage: 2 allocs, 0 frees, 176 bytes allocated
==190672==
==190672== LEAK SUMMARY:
==190672==    definitely lost: 0 bytes in 0 blocks
==190672==    indirectly lost: 0 bytes in 0 blocks
==190672==      possibly lost: 0 bytes in 0 blocks
==190672==    still reachable: 176 bytes in 2 blocks
==190672==         suppressed: 0 bytes in 0 blocks
==190672== Reachable blocks (those to which a pointer was found) are not shown.
==190672== To see them, rerun with: --leak-check=full --show-leak-kinds=all

Some of these leaks are due to OpenSSL/libcurl, not really too much to worry about and besides we cannot do anything about them really.

Larger example

valgrind --leak-check=yes ./result/bin/gcli -t github ci -o mikaelfangel -r nixvim-config -n 5 main
...
==186213== 23 bytes in 1 blocks are definitely lost in loss record 36 of 356
==186213==    at 0x484276B: malloc (in /nix/store/4picxabc7pfzzxk3myzckpifn8dnpvcb-valgrind-3.22.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==186213==    by 0x49E341D: strdup (in /nix/store/qn3ggz5sf3hkjs2c797xf7nan3amdxmp-glibc-2.38-27/lib/libc.so.6)
==186213==    by 0x486B5A5: github_get_checks (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/lib/libgcli.so.0.0.0)
==186213==    by 0x404FA1: github_checks (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x405223: subcommand_ci (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x404C2C: main (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)

Good catch, I am unaware of this one. Will fix :-)

==186213== 33 bytes in 1 blocks are definitely lost in loss record 98 of 356
==186213==    at 0x4849C34: calloc (in /nix/store/4picxabc7pfzzxk3myzckpifn8dnpvcb-valgrind-3.22.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==186213==    by 0x48780AA: sn_asprintf (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/lib/libgcli.so.0.0.0)
==186213==    by 0x405B0B: ensure_config (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x405F6E: gcli_config_find_by_key (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x40656B: gcli_config_get_account (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x406608: gcli_config_get_apibase (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x486B5A5: github_get_checks (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/lib/libgcli.so.0.0.0)
==186213==    by 0x404FA1: github_checks (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x405223: subcommand_ci (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)
==186213==    by 0x404C2C: main (in /nix/store/jryfqyahn63h5xgbhrdz7nqfwsmwvgcn-gcli-2.0.0/bin/gcli)

This one is fixed on trunk I believe. Please try again with the trunk and debug info turned on, this way you get better traces from valgrind. (Note: there's different memory bug on trunk which I'm aware of and will fix soon)

Nico

MikaelFangel commented 11 months ago

Result when building from trunk:

valgrind --leak-check=yes ./result/bin/gcli -t github ci -o mikaelfangel -r nixvim-config -n 5 main
==79187== Memcheck, a memory error detector
==79187== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==79187== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==79187== Command: ./result/bin/gcli -t github ci -o mikaelfangel -r nixvim-config -n 5 main
==79187== 
==79187== Invalid read of size 8
==79187==    at 0x4148D9: add_subcommand_alias (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==    by 0x404C37: main (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==  Address 0x5656268 is 232 bytes inside a block of size 384 free'd
==79187==    at 0x4849E40: realloc (in /nix/store/4picxabc7pfzzxk3myzckpifn8dnpvcb-valgrind-3.22.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==79187==    by 0x4148C5: add_subcommand_alias (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==    by 0x404C37: main (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==  Block was alloc'd at
==79187==    at 0x4849C34: calloc (in /nix/store/4picxabc7pfzzxk3myzckpifn8dnpvcb-valgrind-3.22.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==79187==    by 0x404C00: main (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187== 
ID           STATUS     CONCLUSION  STARTED               COMPLETED             NAME
19025257350  completed  success     2023-11-26T02:54:00Z  2023-11-26T02:56:18Z  tests (mikaelfangel, nixpkgs=channel:nixpkgs-unstable)
19025257314  completed  success     2023-11-26T02:53:58Z  2023-11-26T02:56:12Z  tests (mikaelfangel, nixpkgs=channel:nixos-unstable)
19012259503  completed  success     2023-11-25T02:54:11Z  2023-11-25T02:56:18Z  tests (mikaelfangel, nixpkgs=channel:nixpkgs-unstable)
19012259442  completed  success     2023-11-25T02:54:12Z  2023-11-25T02:56:18Z  tests (mikaelfangel, nixpkgs=channel:nixos-unstable)
18984627698  completed  success     2023-11-24T02:54:15Z  2023-11-24T02:56:22Z  tests (mikaelfangel, nixpkgs=channel:nixpkgs-unstable)
18984627607  completed  success     2023-11-24T02:54:14Z  2023-11-24T02:56:23Z  tests (mikaelfangel, nixpkgs=channel:nixos-unstable)
18958646237  completed  success     2023-11-23T08:24:51Z  2023-11-23T08:26:53Z  tests (mikaelfangel, nixpkgs=channel:nixpkgs-unstable)
18958646039  completed  success     2023-11-23T08:24:52Z  2023-11-23T08:25:07Z  format
18958646038  completed  success     2023-11-23T08:24:49Z  2023-11-23T08:26:51Z  tests (mikaelfangel, nixpkgs=channel:nixos-unstable)
==79187== 
==79187== HEAP SUMMARY:
==79187==     in use at exit: 987,246 bytes in 14,499 blocks
==79187==   total heap usage: 301,885 allocs, 287,386 frees, 12,776,623 bytes allocated
==79187== 
==79187== 33 bytes in 1 blocks are definitely lost in loss record 99 of 359
==79187==    at 0x4849C34: calloc (in /nix/store/4picxabc7pfzzxk3myzckpifn8dnpvcb-valgrind-3.22.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==79187==    by 0x487A6CA: sn_asprintf (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/lib/libgcli.so.0.0.0)
==79187==    by 0x405C1B: ensure_config (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==    by 0x40607D: gcli_config_get_section_entries (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==    by 0x404C47: main (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187== 
==79187== 33 bytes in 1 blocks are definitely lost in loss record 100 of 359
==79187==    at 0x4849C34: calloc (in /nix/store/4picxabc7pfzzxk3myzckpifn8dnpvcb-valgrind-3.22.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==79187==    by 0x487A6CA: sn_asprintf (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/lib/libgcli.so.0.0.0)
==79187==    by 0x405C1B: ensure_config (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==    by 0x4060DE: gcli_config_find_by_key (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==    by 0x4066DB: gcli_config_get_account (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==    by 0x4067BB: gcli_config_get_apibase (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==    by 0x48611B6: gcli_get_apibase (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/lib/libgcli.so.0.0.0)
==79187==    by 0x486CCE5: github_get_checks (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/lib/libgcli.so.0.0.0)
==79187==    by 0x4050B1: github_checks (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==    by 0x405333: subcommand_ci (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187==    by 0x404CEF: main (in /nix/store/9gdgp8cky27azrncq0xpx34ivlx6ydjz-gcli-2.0.0/bin/gcli)
==79187== 
==79187== LEAK SUMMARY:
==79187==    definitely lost: 66 bytes in 2 blocks
==79187==    indirectly lost: 0 bytes in 0 blocks
==79187==      possibly lost: 0 bytes in 0 blocks
==79187==    still reachable: 987,180 bytes in 14,497 blocks
==79187==         suppressed: 0 bytes in 0 blocks
==79187== Reachable blocks (those to which a pointer was found) are not shown.
==79187== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==79187== 
==79187== For lists of detected and suppressed errors, rerun with: -s
==79187== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)
herrhotzenplotz commented 11 months ago

I can't really derive any information from the stack trace because it contains a lot of noise and also it's missing debug info. Are the binaries stripped or are they not even compiled with -g and -O0?

I have tried tracing down the leaks from the functions being called but I am unsure as I can't reproduce the leaks with the same version of valgrind locally (and another machine as well which ran the version 3.21). See the fix/gh-122-leaks branch I just pushed to Github.

Can you please compile without all the Nix Shell noise and include the debug info (see HACKING.md).

MikaelFangel commented 11 months ago

Fair, enough and yes, I actually forgot to compile with the debugging flag on. I'll see if I can set up a debugging environment during the week and provide some better info. (: