The struct mworker_proc is not uniformly freed everywhere, sometimes leading
to leaks of the `id` string (and possibly the other strings).
Introduce a mworker_free_child function instead of duplicating the freeing
logic everywhere to prevent this kind of issues.
This leak was reported in issue #96.
It looks like the leaks have been introduced in commit
9a1ee7ac31c56fd7d881adf2ef4659f336e50c9f,
which is specific to 2.0-dev. Backporting `mworker_free_child` might be
helpful to ease backporting other fixes, though.
---
include/proto/mworker.h | 2 ++
src/haproxy.c | 3 ++-
src/mworker-prog.c | 19 +------------------
src/mworker.c | 30 ++++++++++++++++++++++++++----
4 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/include/proto/mworker.h b/include/proto/mworker.h
index 86f09049f..05fe82af6 100644
--- a/include/proto/mworker.h
+++ b/include/proto/mworker.h
@@ -37,4 +37,6 @@ int mworker_ext_launch_all();
void mworker_kill_max_reloads(int sig);
+void mworker_free_child(struct mworker_proc *);
+
#endif /* PROTO_MWORKER_H_ */
diff --git a/src/haproxy.c b/src/haproxy.c
index a47b7dd32..b688375e2 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -3002,7 +3002,8 @@ int main(int argc, char **argv)
continue;
}
LIST_DEL(&child->list);
- free(child);
+ mworker_free_child(child);
+ child = NULL;
}
}
diff --git a/src/mworker-prog.c b/src/mworker-prog.c
index fd8e66384..467ce9b24 100644
--- a/src/mworker-prog.c
+++ b/src/mworker-prog.c
@@ -69,24 +69,7 @@ int mworker_ext_launch_all()
LIST_DEL(&child->list);
- if (child->command) {
- int i;
-
- for (i = 0; child->command[i]; i++) {
- if (child->command[i]) {
- free(child->command[i]);
- child->command[i] =
NULL;
- }
- }
- free(child->command);
- child->command = NULL;
- }
- if (child->id) {
- free(child->id);
- child->id = NULL;
- }
-
- free(child);
+ mworker_free_child(child);
child = NULL;
continue;
diff --git a/src/mworker.c b/src/mworker.c
index 491d40837..6b814870b 100644
--- a/src/mworker.c
+++ b/src/mworker.c
@@ -185,8 +185,7 @@ void mworker_env_to_proc_list()
if (child->pid) {
LIST_ADDQ(&proc_list, &child->list);
} else {
- free(child->id);
- free(child);
+ mworker_free_child(child);
}
}
@@ -244,7 +243,6 @@ void mworker_catch_sigchld(struct sig_handler *sh)
{
int exitpid = -1;
int status = 0;
- struct mworker_proc *child, *it;
int childfound;
restart_wait:
@@ -253,6 +251,8 @@ restart_wait:
exitpid = waitpid(-1, &status, WNOHANG);
if (exitpid > 0) {
+ struct mworker_proc *child, *it;
+
if (WIFEXITED(status))
status = WEXITSTATUS(status);
else if (WIFSIGNALED(status))
@@ -300,7 +300,8 @@ restart_wait:
ha_warning("Former program '%s' (%d)
exited with code %d (%s)\n", child->id, exitpid, status, (status >= 128) ?
strsignal(status - 128) : "Exit");
}
}
- free(child);
+ mworker_free_child(child);
+ child = NULL;
}
/* do it again to check if it was the last worker */
@@ -553,6 +554,27 @@ out:
return err_code;
}
+void mworker_free_child(struct mworker_proc *child) {
+ if (child == NULL) return;
+
+ if (child->command) {
+ int i;
+
+ for (i = 0; child->command[i]; i++) {
+ if (child->command[i]) {
+ free(child->command[i]);
+ child->command[i] = NULL;
+ }
+ }
+ free(child->command);
+ child->command = NULL;
+ }
+ if (child->id) {
+ free(child->id);
+ child->id = NULL;
+ }
+ free(child);
+}
static struct cfg_kw_list mworker_kws = {{ }, {
{ CFG_GLOBAL, "mworker-max-reloads", mworker_parse_global_max_reloads },
--
2.21.0
