{"thread":{"id":"40684","subject":"[PATCH v2 0/3] daemon: plug memory leak","startedAt":"2015-10-31T09:16:57Z","lastAt":"2015-10-31T20:32:40Z","messageCount":6,"participants":["René Scharfe","Jeff King","Stefan Beller"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"272609","messageId":"56348709.9080207@web.de","threadId":"40684","inReplyTo":null,"subject":"[PATCH v2 0/3] daemon: plug memory leak","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-10-31T09:16:57Z","receivedAt":"2015-10-31T09:16:57Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Changes since v1:\n- Rebased on next, which already has a cleanup function.\n- Added first patch for renaming it.\n\nRene Scharfe (3):\n run-command: name the cleanup function child_process_clear()\n run-command: export child_process_clear()\n daemon: plug memory leak\n\n Documentation/technical/api-run-command.txt | 7 +++++++\n daemon.c | 1 +\n run-command.c | 12 ++++++------\n run-command.h | 1 +\n 4 files changed, 15 insertions(+), 6 deletions(-)\n\n-- \n2.6.2\n"},{"id":"272610","messageId":"56348793.5040909@web.de","threadId":"40684","inReplyTo":"56348709.9080207@web.de","subject":"[PATCH v2 1/3] run-command: name the cleanup function child_process_clear()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-10-31T09:19:15Z","receivedAt":"2015-10-31T09:19:15Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Rename child_process_deinit() to child_process_clear() in order to stay\nconsistent with similar cleanup functions like argv_array_clear(),\nstring_list_clear() etc.\n\nSigned-off-by: Rene Scharfe \n---\n run-command.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 82cc238..b10ec75 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -13,7 +13,7 @@ void child_process_init(struct child_process *child)\n \targv_array_init(&child->env_array);\n }\n \n-static void child_process_deinit(struct child_process *child)\n+static void child_process_clear(struct child_process *child)\n {\n \targv_array_clear(&child->args);\n \targv_array_clear(&child->env_array);\n@@ -335,7 +335,7 @@ int start_command(struct child_process *cmd)\n fail_pipe:\n \t\t\terror(\"cannot create %s pipe for %s: %s\",\n \t\t\t\tstr, cmd->argv[0], strerror(failed_errno));\n-\t\t\tchild_process_deinit(cmd);\n+\t\t\tchild_process_clear(cmd);\n \t\t\terrno = failed_errno;\n \t\t\treturn -1;\n \t\t}\n@@ -520,7 +520,7 @@ fail_pipe:\n \t\t\tclose_pair(fderr);\n \t\telse if (cmd->err)\n \t\t\tclose(cmd->err);\n-\t\tchild_process_deinit(cmd);\n+\t\tchild_process_clear(cmd);\n \t\terrno = failed_errno;\n \t\treturn -1;\n \t}\n@@ -546,7 +546,7 @@ fail_pipe:\n int finish_command(struct child_process *cmd)\n {\n \tint ret = wait_or_whine(cmd->pid, cmd->argv[0], 0);\n-\tchild_process_deinit(cmd);\n+\tchild_process_clear(cmd);\n \treturn ret;\n }\n \n@@ -990,7 +990,7 @@ static void pp_cleanup(struct parallel_processes *pp)\n \n \tfor (i = 0; i < pp->max_processes; i++) {\n \t\tstrbuf_release(&pp->children[i].err);\n-\t\tchild_process_deinit(&pp->children[i].process);\n+\t\tchild_process_clear(&pp->children[i].process);\n \t}\n \n \tfree(pp->children);\n@@ -1157,7 +1157,7 @@ static int pp_collect_finished(struct parallel_processes *pp)\n \t\tpp->nr_processes--;\n \t\tpp->children[i].in_use = 0;\n \t\tpp->pfd[i].fd = -1;\n-\t\tchild_process_deinit(&pp->children[i].process);\n+\t\tchild_process_clear(&pp->children[i].process);\n \t\tchild_process_init(&pp->children[i].process);\n \n \t\tif (i != pp->output_owner) {\n-- \n2.6.2\n"},{"id":"272611","messageId":"563487C6.4050504@web.de","threadId":"40684","inReplyTo":"56348709.9080207@web.de","subject":"[PATCH v2 2/3] run-command: export child_process_clear()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-10-31T09:20:06Z","receivedAt":"2015-10-31T09:20:06Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Make the API symmetric by including a cleanup function as a counterpart\nto child_process_init().\n\nSigned-off-by: Rene Scharfe \n---\n Documentation/technical/api-run-command.txt | 7 +++++++\n run-command.c | 2 +-\n run-command.h | 1 +\n 3 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt\nindex a9fdb45..8bf3e37 100644\n--- a/Documentation/technical/api-run-command.txt\n+++ b/Documentation/technical/api-run-command.txt\n@@ -46,6 +46,13 @@ Functions\n \tThe argument dir corresponds the member .dir. The argument env\n \tcorresponds to the member .env.\n \n+`child_process_clear`::\n+\n+\tRelease the memory associated with the struct child_process.\n+\tMost users of the run-command API don't need to call this\n+\tfunction explicitly because `start_command` invokes it on\n+\tfailure and `finish_command` calls it automatically already.\n+\n The functions above do the following:\n \n . If a system call failed, errno is set and -1 is returned. A diagnostic\ndiff --git a/run-command.c b/run-command.c\nindex b10ec75..0a3c24e 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -13,7 +13,7 @@ void child_process_init(struct child_process *child)\n \targv_array_init(&child->env_array);\n }\n \n-static void child_process_clear(struct child_process *child)\n+void child_process_clear(struct child_process *child)\n {\n \targv_array_clear(&child->args);\n \targv_array_clear(&child->env_array);\ndiff --git a/run-command.h b/run-command.h\nindex 9fe37ee..e296bd2 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -47,6 +47,7 @@ struct child_process {\n \n #define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }\n void child_process_init(struct child_process *);\n+void child_process_clear(struct child_process *);\n \n int start_command(struct child_process *);\n int finish_command(struct child_process *);\n-- \n2.6.2\n"},{"id":"272612","messageId":"563487F4.6080403@web.de","threadId":"40684","inReplyTo":"56348709.9080207@web.de","subject":"[PATCH v2 3/3] daemon: plug memory leak","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-10-31T09:20:52Z","receivedAt":"2015-10-31T09:20:52Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Call child_process_clear() when a child ends to release the memory\nallocated for its environment. This is necessary because unlike all\nother users of start_command() we don't call finish_command(), which\nwould have taken care of that for us.\n\nThis leak was introduced by f063d38b (daemon: use cld->env_array\nwhen re-spawning).\n\nSigned-off-by: Rene Scharfe \n---\n daemon.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/daemon.c b/daemon.c\nindex 56679a1..be70cd4 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -802,6 +802,7 @@ static void check_dead_children(void)\n \t\t\t/* remove the child */\n \t\t\t*cradle = blanket->next;\n \t\t\tlive_children--;\n+\t\t\tchild_process_clear(&blanket->cld);\n \t\t\tfree(blanket);\n \t\t} else\n \t\t\tcradle = &blanket->next;\n-- \n2.6.2\n"},{"id":"272617","messageId":"20151031135304.GA28310@sigill.intra.peff.net","threadId":"40684","inReplyTo":"56348709.9080207@web.de","subject":"Re: [PATCH v2 0/3] daemon: plug memory leak","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-31T13:53:05Z","receivedAt":"2015-10-31T13:53:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 31, 2015 at 10:16:57AM +0100, René Scharfe wrote:\n\n> Changes since v1:\n> - Rebased on next, which already has a cleanup function.\n> - Added first patch for renaming it.\n> \n> Rene Scharfe (3):\n> run-command: name the cleanup function child_process_clear()\n> run-command: export child_process_clear()\n> daemon: plug memory leak\n\nThis round looks good to me. Thanks.\n\n-Peff\n"},{"id":"272639","messageId":"CAGZ79kaXfkmSm0dn40kqrD95D2qpMo+NgEYbs=rVca5qnLTuMQ@mail.gmail.com","threadId":"40684","inReplyTo":"20151031135304.GA28310@sigill.intra.peff.net","subject":"Re: [PATCH v2 0/3] daemon: plug memory leak","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-10-31T20:32:40Z","receivedAt":"2015-10-31T20:32:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Oct 31, 2015 at 6:53 AM, Jeff King wrote:\n> On Sat, Oct 31, 2015 at 10:16:57AM +0100, René Scharfe wrote:\n>\n>> Changes since v1:\n>> - Rebased on next, which already has a cleanup function.\n>> - Added first patch for renaming it.\n>>\n>> Rene Scharfe (3):\n>> run-command: name the cleanup function child_process_clear()\n>> run-command: export child_process_clear()\n>> daemon: plug memory leak\n>\n> This round looks good to me. Thanks.\n\nLooks good to me, too\n\nThanks\nStefan\n\n>\n> -Peff\n"}]}