Re: BUG: Warning: invalid file descriptor -1 in syscall close()

2018-11-26 Thread William Lallemand
On Sun, Nov 25, 2018 at 08:04:14PM +0100, Tim Düsterhus wrote:
> I've taken the usage from your commit message in commit
> e736115d3aaa38d2cfc89fe74174d7e90f4a6976 :-)
> 

Oh right, I edited the usage message in this patch but I forgot to edit the
commit message when I rebased my patches :(

-- 
William Lallemand



Re: BUG: Warning: invalid file descriptor -1 in syscall close()

2018-11-25 Thread Tim Düsterhus
William,

Am 25.11.18 um 19:33 schrieb William Lallemand:
> We can safely add an if > -1 instead of an assert there. So you can edit your
> patch if you want :-)

There should be an updated patch in this thread now.

> 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.
> 

I've taken the usage from your commit message in commit
e736115d3aaa38d2cfc89fe74174d7e90f4a6976 :-)

Best regards
Tim Düsterhus



Re: BUG: Warning: invalid file descriptor -1 in syscall close()

2018-11-25 Thread William Lallemand
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 
>  #include 
>  #include 
>  #include 
> @@ -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



BUG: Warning: invalid file descriptor -1 in syscall close()

2018-11-25 Thread Tim Duesterhus
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 
 #include 
 #include 
 #include 
@@ -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