On Sun, Nov 25, 2018 at 06:30:19PM +0100, Tim Duesterhus wrote:
> Valgrind reports an invalid close of file descriptor -1. After this
> patch haproxy that is started with:
>
> ./haproxy -d -Sa /scratch/haproxy/cli.sock -Ws -f ./haproxy.cfg
>
> aborts in the child process to outline the place where the bug needs
> to be fixed.
>
> Best regards
> ---
> src/haproxy.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/src/haproxy.c b/src/haproxy.c
> index 54ab7c86..e299d7a6 100644
> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -26,6 +26,7 @@
> */
>
> #define _GNU_SOURCE
> +#include <assert.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> @@ -3099,6 +3100,7 @@ int main(int argc, char **argv)
> * workers, we don't need to close the worker
> * side of other workers since it's done with
> * the bind_proc */
> + assert(child->ipc_fd[0] > -1);
> close(child->ipc_fd[0]);
> if (child->relative_pid == relative_pid &&
> child->reloads == 0) {
> --
> 2.19.2
>
>
Hi Tim,
The -1 is a process which has no socketpair initialized. I recently add the
master process in this list, so that's probably this one. You should have
exactly one close(-1) per worker. It is not harmful but we should fix it.
We can safely add an if > -1 instead of an assert there. So you can edit your
patch if you want :-)
By the way, the right option is '-S' and not '-Sa', I know it's not yet on the
documentation file but that's the way it's documented in the usage message.
Thanks,
--
William Lallemand
