Le 22/03/2018 à 20:23, J. Kendzorra a écrit :
So I tried debugging this a little further; in listener.c I added some
debug output around the sections where actconn is in-/decreased.
As light test traffic is running, I can see the following (took a few
hours):
(...)
Increased actconn to 1 (in line 611)
Decreased actconn to 0 (in line 682)
Decreased actconn to -1 (in line 627)
Out of many thousand occurences of modifying actconn, there's a single
instance of actconn getting decreased in line 627, so apparently the
accept returned with an unexpected value. Unfortunately, I missed
getting the output of "show errors" before terminating the process.
Hints on how to debug this better would be appreciated.
Hi,
You're right, there is a bug. But it was not introduced by the commit
you pointed out, but this one: 4f0c64ca ("MINOR: session: release the
listener with the session, not the stream").
I attached the patch to fix this issue. Willy, could you merge it ?
Thanks
--
Christopher Faulet
>From 2355573a959622d2293c522fe1c49c11da589541 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@haproxy.com>
Date: Fri, 23 Mar 2018 15:11:55 +0100
Subject: [PATCH] BUG/MINOR: listener: Don't decrease actconn twice when a new
session is rejected
When a freshly created session is rejected, for any reason, during the accept in
the function "session_accept_fd", the variable "actconn" is decreased twice. The
first time when the rejected session is released, then in the function
"listener_accpect", because of the failure. So it is possible to have an
negative value for actconn. Note that, in this case, we will also have a negatve
value for the current number of connections on the listener rejecting the
session (actconn and l->nbconn are in/decreased in same time).
It is easy to reproduce the bug with this small configuration:
global
stats socket /tmp/haproxy
listen test
bind *:12345
tcp-request connection reject if TRUE
A "show info" on the stat socket, after a connection attempt, will show a very
high value (the unsigned representation of -1).
To fix the bug, if the function "session_accept_fd" returns an error, it
decrements the right counters and "listener_accpect" leaves them untouched.
This patch must be backported in 1.8.
---
src/listener.c | 3 ---
src/session.c | 4 ++++
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/listener.c b/src/listener.c
index 283058ac..20244056 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -617,9 +617,6 @@ void listener_accept(int fd)
* error due to a resource shortage, and we must stop the
* listener (ret < 0).
*/
- if (!(l->options & LI_O_UNLIMITED))
- HA_ATOMIC_SUB(&actconn, 1);
- HA_ATOMIC_SUB(&l->nbconn, 1);
if (ret == 0) /* successful termination */
continue;
diff --git a/src/session.c b/src/session.c
index ae98c947..318c1716 100644
--- a/src/session.c
+++ b/src/session.c
@@ -269,12 +269,16 @@ int session_accept_fd(struct listener *l, int cfd, struct sockaddr_storage *addr
/* error unrolling */
out_free_sess:
+ /* prevent call to listener_release during session_free. It will be
+ * done below, for all errors. */
+ sess->listener = NULL;
session_free(sess);
out_free_conn:
conn_stop_tracking(cli_conn);
conn_xprt_close(cli_conn);
conn_free(cli_conn);
out_close:
+ listener_release(l);
if (ret < 0 && l->bind_conf->xprt == xprt_get(XPRT_RAW) && p->mode == PR_MODE_HTTP) {
/* critical error, no more memory, try to emit a 500 response */
struct chunk *err_msg = &p->errmsg[HTTP_ERR_500];
--
2.14.3
