From: Yeonggi Kim <[email protected]>
Since 5da704b55 ("MEDIUM: log: Update the log applet to use its own
buffers"), syslog_io_handler() leaves early with applet_need_more_data()
when its input buffer is empty:
inbuf = applet_get_inbuf(appctx);
if (inbuf == NULL || !applet_input_data(appctx)) {
goto missing_data;
}
This skips applet_getchar(), which is where the handler notices the
client's shutdown: on an empty buffer it returns -1 once SE_FL_SHW is
set, and the session is closed through cli_abort. Before that commit,
co_getchar() was reached the same way on an empty buffer. Now, when the
shutdown is received alone, after the previous messages were consumed,
the TCP syslog session stays open. A log-forward section does not
inherit "timeout client" from the defaults section, so unless one is set
in the section itself, nothing ever closes the session.
Reproducer:
global
stats socket /tmp/haproxy.sock level admin
log-forward relay
bind 127.0.0.1:1514
log 127.0.0.1:1515 local0
Three clients which each send one message to 127.0.0.1:1514 and close
the connection once it was forwarded leave three sessions in "show sess"
(none with this patch). A soft-stop then waits for "hard-stop-after",
and forever without it. This also happens when a "ring" forwards to a
log-forward section of the same process: on soft-stop the ring's
forwarder closes its connection, the session on the log-forward side
stays, so after a reload the old process is only stopped by
"hard-stop-after".
Only a missing input buffer (not allocated yet) must still make the
handler leave early. An empty one now goes through applet_getchar(),
which returns 0 (wait for more data, as before) or -1 once SE_FL_SHW is
set. The dns and peers applets already work this way: they only test
applet_get_inbuf() for NULL and rely on the getters to report the
shutdown. As before 5da704b55, a closed session is accounted in
cli_aborts, as already done when the client closes in the middle of a
message.
This must be backported as far as 3.3.
---
src/log.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/log.c b/src/log.c
index 3d5dff7..7e2ee16 100644
--- a/src/log.c
+++ b/src/log.c
@@ -6247,10 +6247,13 @@ static void syslog_io_handler(struct appctx *appctx)
goto out;
}
+ /* Only leave early when no input buffer is available. An empty buffer
+ * must still reach applet_getchar() below, because it is what reports
+ * the client's shutdown.
+ */
inbuf = applet_get_inbuf(appctx);
- if (inbuf == NULL || !applet_input_data(appctx)) {
+ if (inbuf == NULL)
goto missing_data;
- }
max_accept = l->bind_conf->maxaccept ? l->bind_conf->maxaccept : 1;
while (1) {
--
2.52.0
