BUG/MINOR: mworker: Fix memory leak of mworker_proc members
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.
diff --git a/include/proto/mworker.h b/include/proto/mworker.h
index 2386a44..0418782 100644
--- a/include/proto/mworker.h
+++ b/include/proto/mworker.h
@@ -36,4 +36,6 @@
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 f5d72e6..923b923 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -3035,7 +3035,8 @@
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 fd8e663..467ce9b 100644
--- a/src/mworker-prog.c
+++ b/src/mworker-prog.c
@@ -69,24 +69,7 @@
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 d5040b7..728a3a1 100644
--- a/src/mworker.c
+++ b/src/mworker.c
@@ -185,9 +185,7 @@
LIST_ADDQ(&proc_list, &child->list);
} else {
- free(child->id);
- free(child);
-
+ mworker_free_child(child);
}
}
@@ -245,7 +243,6 @@
{
int exitpid = -1;
int status = 0;
- struct mworker_proc *child, *it;
int childfound;
restart_wait:
@@ -254,6 +251,8 @@
exitpid = waitpid(-1, &status, WNOHANG);
if (exitpid > 0) {
+ struct mworker_proc *child, *it;
+
if (WIFEXITED(status))
status = WEXITSTATUS(status);
else if (WIFSIGNALED(status))
@@ -301,7 +300,8 @@
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 */
@@ -561,6 +561,29 @@
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 },