From 6118590e9526c826b5087d9a6f6788e98574210a Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Fri, 29 Sep 2023 16:04:21 +0200 Subject: [PATCH] BUG/MINOR: proto_reverse_connect: fix FD leak on connection error Listener using "rev@" address is responsible to setup connection and reverse it using a server instance. If an error occured before reversal is completed, proper freeing must be taken care of by the listener as no session exists for this. Currently, there is two locations where a connection is freed on error before reversal inside reverse_connect protocol. Both of these were incomplete as several function must be used to ensure connection is properly freed. This commit fixes this by reusing the same cleaning mechanism used inside H2 multiplexer. One of the biggest drawback before this patch was that connection FD was not properly removed from fdtab which caused a file-descriptor leak. No need to backport this. --- src/proto_reverse_connect.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/proto_reverse_connect.c b/src/proto_reverse_connect.c index a41921539..1649a69df 100644 --- a/src/proto_reverse_connect.c +++ b/src/proto_reverse_connect.c @@ -92,6 +92,15 @@ static struct connection *new_reverse_conn(struct listener *l, struct server *sr err: if (conn) { + conn_stop_tracking(conn); + conn_xprt_shutw(conn); + conn_xprt_close(conn); + conn_sock_shutw(conn, 0); + conn_ctrl_close(conn); + + if (conn->destroy_cb) + conn->destroy_cb(conn); + /* Mark connection as non-reversable. This prevents conn_free() * to reschedule reverse_connect task on freeing a preconnect * connection. @@ -137,7 +146,14 @@ struct task *rev_process(struct task *task, void *ctx, unsigned int state) if (conn) { if (conn->flags & CO_FL_ERROR) { - conn_full_close(conn); + conn_stop_tracking(conn); + conn_xprt_shutw(conn); + conn_xprt_close(conn); + conn_sock_shutw(conn, 0); + conn_ctrl_close(conn); + + if (conn->destroy_cb) + conn->destroy_cb(conn); conn_free(conn); /* conn_free() must report preconnect failure using rev_notify_preconn_err(). */