From fb76bd5ca647cd30157f6273bb69cdfb2075f520 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 24 Sep 2020 18:07:48 +0200 Subject: [PATCH] BUG/MEDIUM: listeners: correctly report pause() errors By using the same "ret" variable in the "if" block to test the return value of pause(), the second one shadows the first one and when forcing the result to zero in case of an error, it doesn't do anything. The problem is that some listeners used to fail to pause in multi-process mode and this was not reported, but their failure was automatically resolved by the last process to pause. By properly checking for errors we might now possibly report a race once in a while so we may have to roll this back later if some users meet it. The test on ==0 is wrong too since technically speaking a total stop validates the need for a pause, but stops the listener so it's just the resume that won't work anymore. We could switch to stopped but it's an involuntary switch and the user will not know. Better then mark it as paused and let the resume continue to fail so that only the resume will eventually report an error (e.g. abns@). This must not be backported as there is a risk of side effect by fixing this bug, given that it hides other bugs itself. --- src/listener.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/listener.c b/src/listener.c index 85158e4438..b2a6862ca9 100644 --- a/src/listener.c +++ b/src/listener.c @@ -351,14 +351,13 @@ int pause_listener(struct listener *l) /* Returns < 0 in case of failure, 0 if the listener * was totally stopped, or > 0 if correctly paused. */ - int ret = l->rx.proto->pause(l); + ret = l->rx.proto->pause(l); if (ret < 0) { ret = 0; goto end; } - else if (ret == 0) - goto end; + ret = 1; } MT_LIST_DEL(&l->wait_queue); -- 2.39.5