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

[PATCH v2 05/23] builtin/submodule--helper: clear child process when not running it

From
Patrick Steinhardt <ps@pks.im>
Date
Sep 26, 2024, 11:46 UTC
Message-ID
<0d0964a2bec7c6e6375a65a5ea9dc79a18ad325c.1727351062.git.ps@pks.im>
In-Reply-To
<cover.1727351062.git.ps@pks.im>

In `runcommand_in_submodule_cb()` we may end up not executing the child command when `argv` is empty. But we still populate the command with environment variables and other things, which needs cleanup. This leads to a memory leak because we do not call `finish_command()`.

Fix this by clearing the child process when we don't execute it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/submodule--helper.c  | 10 +++++++---
 t/t7407-submodule-foreach.sh |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index ed05dc5134..fd1b679408 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -363,9 +363,13 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item,
 	if (!info->quiet)
 		printf(_("Entering '%s'\n"), displaypath);
 
-	if (info->argv[0] && run_command(&cp))
-		die(_("run_command returned non-zero status for %s\n."),
-			displaypath);
+	if (info->argv[0]) {
+		if (run_command(&cp))
+			die(_("run_command returned non-zero status for %s\n."),
+			    displaypath);
+	} else {
+		child_process_clear(&cp);
+	}
 
 	if (info->recursive) {
 		struct child_process cpr = CHILD_PROCESS_INIT;
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 8d7b234beb..9f68893261 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -12,6 +12,7 @@ that are currently checked out.
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 
-- 
2.46.2.852.g229c0bf0e5.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 44 of 62 in “Memory leak fixes (pt.7)”
  1. 00/23 Memory leak fixes (pt.7)Patrick Steinhardt, Sep 16, 2024
  2. 01/23 builtin/help: fix dangling reference to `html_path`Patrick Steinhardt, Sep 16, 2024
  3. Justin ToblerSep 16, 2024
  4. 02/23 builtin/help: fix leaking `html_path` when reading config multiple timesPatrick Steinhardt, Sep 16, 2024
  5. 03/23 git: fix leaking argv when handling builtinsPatrick Steinhardt, Sep 16, 2024
  6. 04/23 submodule: fix leaking update strategyPatrick Steinhardt, Sep 16, 2024
  7. 05/23 builtin/submodule--helper: clear child process when not running itPatrick Steinhardt, Sep 16, 2024
  8. 06/23 builtin/submodule--helper: fix leaking error bufferPatrick Steinhardt, Sep 16, 2024
  9. 07/23 t/helper: fix leaking subrepo in nested submodule config helperPatrick Steinhardt, Sep 16, 2024
  10. 08/23 builtin/submodule--helper: fix leaking remote ref on errorsPatrick Steinhardt, Sep 16, 2024
  11. Justin ToblerSep 16, 2024
  12. Patrick SteinhardtSep 17, 2024
  13. Junio C HamanoSep 25, 2024
  14. Patrick SteinhardtSep 26, 2024
  15. 09/23 dir: fix off by one errors for ignored and untracked entriesPatrick Steinhardt, Sep 16, 2024
  16. Junio C HamanoSep 20, 2024
  17. 10/23 builtin/pull: fix leaking "ff" optionPatrick Steinhardt, Sep 16, 2024
  18. Junio C HamanoSep 20, 2024
  19. Patrick SteinhardtSep 24, 2024
  20. 11/23 diff: fix leaking orderfile optionPatrick Steinhardt, Sep 16, 2024
  21. 12/23 parse-options: free previous value of `OPTION_FILENAME`Patrick Steinhardt, Sep 16, 2024
  22. Junio C HamanoSep 20, 2024
  23. 13/23 diffcore-order: fix leaking buffer when parsing orderfilesPatrick Steinhardt, Sep 16, 2024
  24. 14/23 builtin/repack: fix leaking configurationPatrick Steinhardt, Sep 16, 2024
  25. Junio C HamanoSep 20, 2024
  26. 15/23 builtin/difftool: plug several trivial memory leaksPatrick Steinhardt, Sep 16, 2024
  27. 16/23 trace2: destroy context stored in thread-local storagePatrick Steinhardt, Sep 16, 2024
  28. 17/23 submodule: fix leaking submodule ODB pathsPatrick Steinhardt, Sep 16, 2024
  29. 18/23 grep: fix leaking grep patternPatrick Steinhardt, Sep 16, 2024
  30. 19/23 promisor-remote: fix leaking partial clone filterPatrick Steinhardt, Sep 16, 2024
  31. 20/23 builtin/maintenance: fix leaking config stringPatrick Steinhardt, Sep 16, 2024
  32. Junio C HamanoSep 20, 2024
  33. 21/23 builtin/maintenance: fix leak in `get_schedule_cmd()`Patrick Steinhardt, Sep 16, 2024
  34. 22/23 revision: fix leaking parents when simplifying commitsPatrick Steinhardt, Sep 16, 2024
  35. Junio C HamanoSep 19, 2024
  36. 23/23 diffcore-break: fix leaking filespecs when merging broken pairsPatrick Steinhardt, Sep 16, 2024
  37. Junio C HamanoSep 19, 2024
  38. Patrick SteinhardtSep 24, 2024
  39. 00/23 Memory leak fixes (pt.7)Patrick Steinhardt, Sep 26, 2024
  40. 01/23 builtin/help: fix dangling reference to `html_path`Patrick Steinhardt, Sep 26, 2024
  41. 02/23 builtin/help: fix leaking `html_path` when reading config multiple timesPatrick Steinhardt, Sep 26, 2024
  42. 03/23 git: fix leaking argv when handling builtinsPatrick Steinhardt, Sep 26, 2024
  43. 04/23 submodule: fix leaking update strategyPatrick Steinhardt, Sep 26, 2024
  44. 05/23 builtin/submodule--helper: clear child process when not running itPatrick Steinhardt, Sep 26, 2024
  45. 06/23 builtin/submodule--helper: fix leaking error bufferPatrick Steinhardt, Sep 26, 2024
  46. 07/23 t/helper: fix leaking subrepo in nested submodule config helperPatrick Steinhardt, Sep 26, 2024
  47. 08/23 builtin/submodule--helper: fix leaking remote ref on errorsPatrick Steinhardt, Sep 26, 2024
  48. 09/23 dir: fix off by one errors for ignored and untracked entriesPatrick Steinhardt, Sep 26, 2024
  49. 10/23 builtin/pull: fix leaking "ff" optionPatrick Steinhardt, Sep 26, 2024
  50. 11/23 diff: fix leaking orderfile optionPatrick Steinhardt, Sep 26, 2024
  51. 12/23 parse-options: free previous value of `OPTION_FILENAME`Patrick Steinhardt, Sep 26, 2024
  52. 13/23 diffcore-order: fix leaking buffer when parsing orderfilesPatrick Steinhardt, Sep 26, 2024
  53. 14/23 builtin/repack: fix leaking configurationPatrick Steinhardt, Sep 26, 2024
  54. 15/23 builtin/difftool: plug several trivial memory leaksPatrick Steinhardt, Sep 26, 2024
  55. 16/23 trace2: destroy context stored in thread-local storagePatrick Steinhardt, Sep 26, 2024
  56. 17/23 submodule: fix leaking submodule ODB pathsPatrick Steinhardt, Sep 26, 2024
  57. 18/23 grep: fix leaking grep patternPatrick Steinhardt, Sep 26, 2024
  58. 19/23 promisor-remote: fix leaking partial clone filterPatrick Steinhardt, Sep 26, 2024
  59. 20/23 builtin/maintenance: fix leaking config stringPatrick Steinhardt, Sep 26, 2024
  60. 21/23 builtin/maintenance: fix leak in `get_schedule_cmd()`Patrick Steinhardt, Sep 26, 2024
  61. 22/23 revision: fix leaking parents when simplifying commitsPatrick Steinhardt, Sep 26, 2024
  62. 23/23 diffcore-break: fix leaking filespecs when merging broken pairsPatrick Steinhardt, Sep 26, 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.