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 9a1ee7ac31,
which is specific to 2.0-dev. Backporting `mworker_free_child` might be
helpful to ease backporting other fixes, though.
This commit is contained in:
Tim Duesterhus 2019-05-16 20:23:22 +02:00 committed by William Lallemand
parent 80daaa1e9d
commit 9b7a976cd6
4 changed files with 33 additions and 24 deletions

View File

@ -36,4 +36,6 @@ int mworker_ext_launch_all();
void mworker_kill_max_reloads(int sig);
void mworker_free_child(struct mworker_proc *);
#endif /* PROTO_MWORKER_H_ */

View File

@ -3035,7 +3035,8 @@ int main(int argc, char **argv)
continue;
}
LIST_DEL(&child->list);
free(child);
mworker_free_child(child);
child = NULL;
}
}

View File

@ -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;

View File

@ -185,9 +185,7 @@ void mworker_env_to_proc_list()
LIST_ADDQ(&proc_list, &child->list);
} else {
free(child->id);
free(child);
mworker_free_child(child);
}
}
@ -245,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:
@ -254,6 +251,8 @@ void mworker_catch_sigchld(struct sig_handler *sh)
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 @@ void mworker_catch_sigchld(struct sig_handler *sh)
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 @@ static int mworker_parse_global_max_reloads(char **args, int section_type, struc
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 },