]> git.ipfire.org Git - thirdparty/haproxy.git/commit
BUG/MEDIUM: connections: permit to permanently remove an idle conn
authorWilly Tarreau <w@1wt.eu>
Wed, 5 Nov 2025 09:51:27 +0000 (10:51 +0100)
committerWilly Tarreau <w@1wt.eu>
Wed, 5 Nov 2025 10:08:25 +0000 (11:08 +0100)
commit096999ee208b8ae306983bc3fd677517d05948d2
tree142c7daaa50f16a7861daed4d209817aa62b366b
parent59c599f3f07c549c195f1bd877d2c2a09e6bc61f
BUG/MEDIUM: connections: permit to permanently remove an idle conn

There's currently a function conn_delete_from_tree() which is used to
detach an idle connection from the tree it's currently attached to so
that it is no longer found. This function is used in three circumstances:
  - when picking a new connection that no longer has any avail stream
  - when temporarily working on the connection from an I/O handler,
    in which case it's re-added at the end
  - when killing a connection

The 2nd case above is quite specific, as it requires to preserve the
CO_FL_LIST_MASK flags so that the connection can be re-inserted into
the proper tree when leaving the handler. However, there's a catch.
When killing a connection, we want to be certain it will not be
reinserted into the tree. The flags preservation is causing a tiny
race if an I/O happens while the connection is in the kill list,
because in this case the I/O handler will note the connection flags,
do its work, then reinsert the connection where it believed it was,
then the connection gets purged, and another user can find it in the
tree.

The issue is very difficult to reproduce. On a 128-thread machine it
happens in H2 around 500k req/s after around 50M requests. In H1 it
happens after around 1 billion requests.

The fix here consists in passing an extra argument to the function to
indicate if the removal is permanent or not. When it's permanent, the
function will clear the associated flags. The callers were adjusted
so that all those dequeuing a connection in order to kill it do it
permanently and all other ones do it only temporarily.

A slightly different approach could have worked: the function could
always remove all flags, and the callers would need to restore them.
But this would require trickier modifications of the various call
places, compared to only passing 0/1 to indicate the permanent status.

This will need to be backported to all stable versions. The issue was
at least reproduced since 3.1 (not tested before). The patch will need
to be adjusted for 3.2 and older, because a 2nd argument "thr" was
added in 3.3, so the patch will not apply to older versions as-is.
include/haproxy/connection.h
src/backend.c
src/connection.c
src/mux_fcgi.c
src/mux_h1.c
src/mux_h2.c
src/mux_quic.c
src/mux_spop.c
src/server.c
src/ssl_sock.c