git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 00/21] Various memory leak fixes

From
Karthik Nayak <karthik.188@gmail.com>
Date
May 29, 2024, 08:25 UTC
Message-ID
<CAOLa=ZQZ2ap9rgS-u0K9LsoZVfZ5hDPLY1ALZDSnfNmQXx35Bg@mail.gmail.com>
In-Reply-To
<ZlQr11P_C-KyO2DF@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
Show 51 quoted lines
> On Fri, May 24, 2024 at 07:10:09PM -0700, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>>
>> > this is the second version of my patch series that fixes various memory
>> > leaks in Git. Changes compared to v1:
>> >
>> >   - t4153 and t7006 aren't marked as passing anymore. I thought they
>> >     pass because most of these tests were skipped because of a missing
>> >     TTY prerequisite both on my local machine, but also in our CI.
>> >
>> >   - Add another patch to install the Perl IO:Pty module on Alpine and
>> >     Ubuntu. This fulfills the TTY prerequisite and thus surfaces the
>> >     memory leaks in both of the above tests.
>> >
>> >   - Add another unit test for strvec that exercise replacing a string in
>> >     the strvec with a copy of itself.
>> >
>> >   - A bunch of commit message improvements.
>>
>> Looking very good.  This seems to reveal existing leaks when merged
>> to 'seen'; other topics that are not in 'master' may be introducing
>> these leaks.  I'll see if a trial merge to 'next' is leak-free (in
>> which case I'll merge it down to 'next') or there are other topics
>> in 'next' that are leaking (in which case we'll play by ear---either
>> mark the tests again as non-leak-free, or plug the leak if it seems
>> trivial).
>>
>>  https://github.com/git/git/actions/runs/9231313414/job/25400998823
>>
>> says t1400-update-ref has many "stdin symref-update" things are
>> failing.
>
> Indeed. The following diff fixes the leak:
>
>     diff --git a/builtin/update-ref.c b/builtin/update-ref.c
>     index 7d2a419230..e54be9c429 100644
>     --- a/builtin/update-ref.c
>     +++ b/builtin/update-ref.c
>     @@ -130,6 +130,8 @@ static char *parse_next_arg(const char **next)
>
>         if (arg.len)
>             return strbuf_detach(&arg, NULL);
>     +
>     +	strbuf_release(&arg);
>         return NULL;
>      }
>
>
> Karthik is out of office this week, so you may want to add this as a
> "SQUASH???" commit on top of his topic branch to make "seen" pass.
>

Thanks Patrick. Indeed I'm a bit slow on my responses, since I'm on vacation, but yeah, I too came about adding this as a fix.

I'll mostly check for all tests and send a new version based on this series soon.

Show 13 quoted lines
>> Also
>>
>>  https://github.com/git/git/actions/runs/9231313414/job/25401102951
>>
>> shows that t1460-refs-migrate fails on Windows.
>
> Hm, this one is curious. There are no leak logs at all, and the exit
> code is 139. Might be SIGSEGV, indicating that something else is going
> on here than a memory leak.
>
> I'll investigate.
>
> Patrick
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 76 of 115 in “Various memory leak fixes”
  1. 00/20 Various memory leak fixesPatrick Steinhardt, May 23, 2024
  2. 01/20 t: mark a bunch of tests as leak-freePatrick Steinhardt, May 23, 2024
  3. Junio C HamanoMay 23, 2024
  4. Patrick SteinhardtMay 24, 2024
  5. Junio C HamanoMay 24, 2024
  6. Junio C HamanoMay 24, 2024
  7. Karthik NayakMay 24, 2024
  8. 02/20 transport-helper: fix leaking helper namePatrick Steinhardt, May 23, 2024
  9. Junio C HamanoMay 23, 2024
  10. Karthik NayakMay 24, 2024
  11. 03/20 strbuf: fix leak when `appendwholeline()` fails with EOFPatrick Steinhardt, May 23, 2024
  12. 04/20 checkout: clarify memory ownership in `unique_tracking_name()`Patrick Steinhardt, May 23, 2024
  13. 05/20 http: refactor code to clarify memory ownershipPatrick Steinhardt, May 23, 2024
  14. 06/20 config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt, May 23, 2024
  15. 07/20 diff: refactor code to clarify memory ownership of prefixesPatrick Steinhardt, May 23, 2024
  16. Eric SunshineMay 23, 2024
  17. 08/20 convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt, May 23, 2024
  18. 09/20 builtin/log: stop using globals for log configPatrick Steinhardt, May 23, 2024
  19. 10/20 builtin/log: stop using globals for format configPatrick Steinhardt, May 23, 2024
  20. 11/20 config: clarify memory ownership in `git_config_string()`Patrick Steinhardt, May 23, 2024
  21. 12/20 config: plug various memory leaksPatrick Steinhardt, May 23, 2024
  22. Junio C HamanoMay 23, 2024
  23. Patrick SteinhardtMay 24, 2024
  24. Patrick SteinhardtMay 24, 2024
  25. Junio C HamanoMay 24, 2024
  26. Junio C HamanoMay 24, 2024
  27. 13/20 builtin/credential: clear credential before exitPatrick Steinhardt, May 23, 2024
  28. 14/20 commit-reach: fix memory leak in `ahead_behind()`Patrick Steinhardt, May 23, 2024
  29. 15/20 submodule: fix leaking memory for submodule entriesPatrick Steinhardt, May 23, 2024
  30. 16/20 strvec: add functions to replace and remove stringsPatrick Steinhardt, May 23, 2024
  31. Eric SunshineMay 23, 2024
  32. Patrick SteinhardtMay 24, 2024
  33. 17/20 builtin/mv: refactor `add_slash()` to always return allocated stringsPatrick Steinhardt, May 23, 2024
  34. 18/20 builtin/mv duplicate string list memoryPatrick Steinhardt, May 23, 2024
  35. 19/20 builtin/mv: refactor to use `struct strvec`Patrick Steinhardt, May 23, 2024
  36. 20/20 builtin/mv: fix leaks for submodule gitfile pathsPatrick Steinhardt, May 23, 2024
  37. Junio C HamanoMay 23, 2024
  38. Patrick SteinhardtMay 24, 2024
  39. 00/21 Various memory leak fixesPatrick Steinhardt, May 24, 2024
  40. 01/21 ci: add missing dependency for TTY prereqPatrick Steinhardt, May 24, 2024
  41. Junio C HamanoMay 24, 2024
  42. 02/21 t: mark a bunch of tests as leak-freePatrick Steinhardt, May 24, 2024
  43. 03/21 transport-helper: fix leaking helper namePatrick Steinhardt, May 24, 2024
  44. 04/21 strbuf: fix leak when `appendwholeline()` fails with EOFPatrick Steinhardt, May 24, 2024
  45. Jeff KingMay 25, 2024
  46. Patrick SteinhardtMay 27, 2024
  47. Jeff KingMay 29, 2024
  48. Patrick SteinhardtMay 29, 2024
  49. Jeff KingMay 30, 2024
  50. 05/21 checkout: clarify memory ownership in `unique_tracking_name()`Patrick Steinhardt, May 24, 2024
  51. 06/21 http: refactor code to clarify memory ownershipPatrick Steinhardt, May 24, 2024
  52. 07/21 config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt, May 24, 2024
  53. 08/21 diff: refactor code to clarify memory ownership of prefixesPatrick Steinhardt, May 24, 2024
  54. 09/21 convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt, May 24, 2024
  55. 10/21 builtin/log: stop using globals for log configPatrick Steinhardt, May 24, 2024
  56. 11/21 builtin/log: stop using globals for format configPatrick Steinhardt, May 24, 2024
  57. 12/21 config: clarify memory ownership in `git_config_string()`Patrick Steinhardt, May 24, 2024
  58. 13/21 config: plug various memory leaksPatrick Steinhardt, May 24, 2024
  59. Patrick SteinhardtMay 24, 2024
  60. Jeff KingMay 25, 2024
  61. Patrick SteinhardtMay 27, 2024
  62. Jeff KingMay 29, 2024
  63. 14/21 builtin/credential: clear credential before exitPatrick Steinhardt, May 24, 2024
  64. 15/21 commit-reach: fix memory leak in `ahead_behind()`Patrick Steinhardt, May 24, 2024
  65. 16/21 submodule: fix leaking memory for submodule entriesPatrick Steinhardt, May 24, 2024
  66. 17/21 strvec: add functions to replace and remove stringsPatrick Steinhardt, May 24, 2024
  67. 18/21 builtin/mv: refactor `add_slash()` to always return allocated stringsPatrick Steinhardt, May 24, 2024
  68. 19/21 builtin/mv duplicate string list memoryPatrick Steinhardt, May 24, 2024
  69. 20/21 builtin/mv: refactor to use `struct strvec`Patrick Steinhardt, May 24, 2024
  70. 21/21 builtin/mv: fix leaks for submodule gitfile pathsPatrick Steinhardt, May 24, 2024
  71. Junio C HamanoMay 25, 2024
  72. Patrick SteinhardtMay 27, 2024
  73. Junio C HamanoMay 27, 2024
  74. Junio C HamanoMay 27, 2024
  75. Patrick SteinhardtMay 28, 2024
  76. Karthik NayakMay 29, 2024
  77. 00/21 Various memory leak fixesPatrick Steinhardt, May 27, 2024
  78. 01/21 ci: add missing dependency for TTY prereqPatrick Steinhardt, May 27, 2024
  79. 02/21 t: mark a bunch of tests as leak-freePatrick Steinhardt, May 27, 2024
  80. 03/21 transport-helper: fix leaking helper namePatrick Steinhardt, May 27, 2024
  81. 04/21 strbuf: fix leak when `appendwholeline()` fails with EOFPatrick Steinhardt, May 27, 2024
  82. 05/21 checkout: clarify memory ownership in `unique_tracking_name()`Patrick Steinhardt, May 27, 2024
  83. 06/21 http: refactor code to clarify memory ownershipPatrick Steinhardt, May 27, 2024
  84. 07/21 config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt, May 27, 2024
  85. 08/21 diff: refactor code to clarify memory ownership of prefixesPatrick Steinhardt, May 27, 2024
  86. 09/21 convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt, May 27, 2024
  87. 10/21 builtin/log: stop using globals for log configPatrick Steinhardt, May 27, 2024
  88. 11/21 builtin/log: stop using globals for format configPatrick Steinhardt, May 27, 2024
  89. 12/21 config: clarify memory ownership in `git_config_string()`Patrick Steinhardt, May 27, 2024
  90. 13/21 config: plug various memory leaksPatrick Steinhardt, May 27, 2024
  91. 14/21 builtin/credential: clear credential before exitPatrick Steinhardt, May 27, 2024
  92. 15/21 commit-reach: fix memory leak in `ahead_behind()`Patrick Steinhardt, May 27, 2024
  93. 16/21 submodule: fix leaking memory for submodule entriesPatrick Steinhardt, May 27, 2024
  94. 17/21 strvec: add functions to replace and remove stringsPatrick Steinhardt, May 27, 2024
  95. 18/21 builtin/mv: refactor `add_slash()` to always return allocated stringsPatrick Steinhardt, May 27, 2024
  96. 19/21 builtin/mv duplicate string list memoryPatrick Steinhardt, May 27, 2024
  97. 20/21 builtin/mv: refactor to use `struct strvec`Patrick Steinhardt, May 27, 2024
  98. 21/21 builtin/mv: fix leaks for submodule gitfile pathsPatrick Steinhardt, May 27, 2024
  99. Junio C HamanoMay 27, 2024
  100. 0/5 add-ons for ps/leakfixesJeff King, May 30, 2024
  101. 1/5 t-strvec: use va_end() to match va_start()Jeff King, May 30, 2024
  102. 2/5 t-strvec: mark variable-arg helper with LAST_ARG_MUST_BE_NULLJeff King, May 30, 2024
  103. 3/5 mv: move src_dir cleanup to end of cmd_mv()Jeff King, May 30, 2024
  104. Patrick SteinhardtMay 30, 2024
  105. Jeff KingMay 30, 2024
  106. Patrick SteinhardtMay 30, 2024
  107. Jeff KingMay 30, 2024
  108. Patrick SteinhardtMay 30, 2024
  109. Jeff KingMay 30, 2024
  110. 4/5 mv: factor out empty src_dir removalJeff King, May 30, 2024
  111. 5/5 mv: replace src_dir with a strvecJeff King, May 30, 2024
  112. Junio C HamanoMay 30, 2024
  113. Jeff KingMay 31, 2024
  114. Junio C HamanoMay 31, 2024
  115. Patrick SteinhardtMay 30, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.