BUG/MEDIUM: log: do not ignore the peer's shutdown in the syslog applet

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.

diff --git a/src/log.c b/src/log.c
index 3d5dff7b16a8885badec587497b0644b1c5a39e2..7e2ee16ec03d9326ff3ee610fc546884658bcbfb 100644 (file)
--- 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) {