Re: exit error regression on haproxy1.8dev

[Willy Tarreau]

On Thu, Jun 08, 2017 at 08:37:28PM +0200, William Lallemand wrote:
> On Thu, Jun 08, 2017 at 01:49:15PM +0200, William Lallemand wrote:
> >  
> > You are right about the regression, however this part of the code should be
> > called in daemon mode only, the regression was elsewhere :)
> > 
> > Fix attached.
> > 
> 
> Willy, could you apply this one?

sure, now done, thanks.
Willy

Re: exit error regression on haproxy1.8dev

[William Lallemand]

On Thu, Jun 08, 2017 at 01:49:15PM +0200, William Lallemand wrote:
>  
> You are right about the regression, however this part of the code should be
> called in daemon mode only, the regression was elsewhere :)
> 
> Fix attached.
> 

Willy, could you apply this one?

Thanks,

-- 
William Lallemand

Re: exit error regression on haproxy1.8dev

[William Lallemand]

On Wed, Jun 07, 2017 at 02:13:58PM +0200, Emmanuel Hocdet wrote:
> 
> > Le 7 juin 2017 à 10:34, Emmanuel Hocdet  a écrit :
> > 
> > Hi,
> > 
> > ok:
> >> haproxy -f test.cfg -W
> >> echo $?
> > 0
> > 
> > bad:
> >> haproxy -f test.cfg
> >> echo $?
> > 1
> > 
> > With bunch of modified code, i am a little lost to track the bug.
> > 
> > Manu
> > 
> > 
> 
 
You are right about the regression, however this part of the code should be
called in daemon mode only, the regression was elsewhere :)

Fix attached.

-- 
William Lallemand
>From 4171380e2ff3d562bfcf4f7aa5325d79ccda025b Mon Sep 17 00:00:00 2001
From: William Lallemand 
Date: Wed, 7 Jun 2017 15:04:47 +0200
Subject: [PATCH] BUG/MEDIUM: misplaced exit and wrong exit code

Commit cb11fd2 ("MEDIUM: mworker: wait mode on reload failure")
introduced a regression, when HAProxy is used in daemon mode, it exits 1
after forking its children.

HAProxy should exit(0), the exit(EXIT_FAILURE) was expected to be use
when the master fail in master-worker mode.

Thanks to Emmanuel Hocdet for reporting this bug. No backport needed.
---
 src/haproxy.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 60bd334b..51cdf48e 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2574,12 +2574,13 @@ int main(int argc, char **argv)
 			if (global.mode & MODE_MWORKER) {
 protocol_unbind_all();
 mworker_wait();
+/* should never get there */
+exit(EXIT_FAILURE);
 			}
 #ifndef OPENSSL_NO_DH
 			ssl_free_dh();
 #endif
-			/* should never get there */
-			exit(EXIT_FAILURE);
+			exit(0); /* parent must leave */
 		}
 
 		/* child must never use the atexit function */
-- 
2.13.0

Re: exit error regression on haproxy1.8dev

[Emmanuel Hocdet]

> Le 7 juin 2017 à 15:20, William Lallemand  a écrit :
> 
> On Wed, Jun 07, 2017 at 02:13:58PM +0200, Emmanuel Hocdet wrote:
>> 
>>> Le 7 juin 2017 à 10:34, Emmanuel Hocdet  a écrit :
>>> 
>>> Hi,
>>> 
>>> ok:
 haproxy -f test.cfg -W
 echo $?
>>> 0
>>> 
>>> bad:
 haproxy -f test.cfg
 echo $?
>>> 1
>>> 
>>> With bunch of modified code, i am a little lost to track the bug.
>>> 
>>> Manu
>>> 
>>> 
>> 
> 
> You are right about the regression, however this part of the code should be
> called in daemon mode only, the regression was elsewhere :)
> 
> Fix attached.
> 
> -- 
> William Lallemand
> <0001-BUG-MEDIUM-misplaced-exit-and-wrong-exit-code.patch>

test ok for me

thanks!

Re: exit error regression on haproxy1.8dev

[William Lallemand]

On Wed, Jun 07, 2017 at 02:13:58PM +0200, Emmanuel Hocdet wrote:
> 
> > Le 7 juin 2017 à 10:34, Emmanuel Hocdet  a écrit :
> > 
> > Hi,
> > 
> > ok:
> >> haproxy -f test.cfg -W
> >> echo $?
> > 0
> > 
> > bad:
> >> haproxy -f test.cfg
> >> echo $?
> > 1
> > 
> > With bunch of modified code, i am a little lost to track the bug.
> > 
> > Manu
> > 
> > 
> 
 
You are right about the regression, however this part of the code should be
called in daemon mode only, the regression was elsewhere :)

Fix attached.

-- 
William Lallemand
>From 4171380e2ff3d562bfcf4f7aa5325d79ccda025b Mon Sep 17 00:00:00 2001
From: William Lallemand 
Date: Wed, 7 Jun 2017 15:04:47 +0200
Subject: [PATCH] BUG/MEDIUM: misplaced exit and wrong exit code

Commit cb11fd2 ("MEDIUM: mworker: wait mode on reload failure")
introduced a regression, when HAProxy is used in daemon mode, it exits 1
after forking its children.

HAProxy should exit(0), the exit(EXIT_FAILURE) was expected to be use
when the master fail in master-worker mode.

Thanks to Emmanuel Hocdet for reporting this bug. No backport needed.
---
 src/haproxy.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 60bd334b..51cdf48e 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2574,12 +2574,13 @@ int main(int argc, char **argv)
 			if (global.mode & MODE_MWORKER) {
 protocol_unbind_all();
 mworker_wait();
+/* should never get there */
+exit(EXIT_FAILURE);
 			}
 #ifndef OPENSSL_NO_DH
 			ssl_free_dh();
 #endif
-			/* should never get there */
-			exit(EXIT_FAILURE);
+			exit(0); /* parent must leave */
 		}
 
 		/* child must never use the atexit function */
-- 
2.13.0

Re: exit error regression on haproxy1.8dev

[Emmanuel Hocdet]

> Le 7 juin 2017 à 10:34, Emmanuel Hocdet  a écrit :
> 
> Hi,
> 
> ok:
>> haproxy -f test.cfg -W
>> echo $?
> 0
> 
> bad:
>> haproxy -f test.cfg
>> echo $?
> 1
> 
> With bunch of modified code, i am a little lost to track the bug.
> 
> Manu
> 
> 


This patch seem correct the problem, but…

diff --git a/src/haproxy.c b/src/haproxy.c
index 60bd334..1cc39b0 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2495,9 +2495,9 @@ int main(int argc, char **argv)
 * process live in background before forking children
 */
 
-   if ((getenv("HAPROXY_MWORKER_REEXEC") == NULL)
-   && (global.mode & MODE_MWORKER)
-   && (global.mode & MODE_DAEMON)) {
+   if (((getenv("HAPROXY_MWORKER_REEXEC") == NULL)
+&& (global.mode & MODE_MWORKER)  && (global.mode & 
MODE_DAEMON))
+   || (!(global.mode & MODE_MWORKER) && (global.mode & 
MODE_DAEMON))) {
ret = fork();
if (ret < 0) {
Alert("[%s.main()] Cannot fork.\n", argv[0]);

exit error regression on haproxy1.8dev

[Emmanuel Hocdet]

Hi,

ok:
> haproxy -f test.cfg -W
> echo $?
0

bad:
> haproxy -f test.cfg
> echo $?
1

With bunch of modified code, i am a little lost to track the bug.

Manu