Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

segfault when reloading HAProxy if there are many persistent connections #2184

Closed
sigint2 opened this issue Jun 18, 2023 · 4 comments
Closed
Labels
status: fixed This issue is a now-fixed bug. type: bug This issue describes a bug.

Comments

@sigint2
Copy link

sigint2 commented Jun 18, 2023

Detailed Description of the Problem

In master-worker mode, the HAProxy master seems to reliably segfault if I send it many reloads (~250) with SIGUSR2, and if in between the reloads I create a fresh long-lived connection. I've reproduced on a couple of different systems (smaller VMs to bigger hardware).

The bug is the below:

FATAL: bug condition "fd >= global.maxsock" matched at include/haproxy/fd.h:437
  call trace(7):
  | 0x55711b5e07e7 [c6 04 25 01 00 00 00 00]: main-0xa39
  | 0x55711b735985 [85 c0 0f 84 6b 02 00 00]: main+0x154765
  | 0x55711b5e346e [e9 eb ee ff ff e8 c8 dc]: main+0x224e/0x2cbe

Segmentation fault (core dumped)

Expected Behavior

I know the reloads are costly so while I was not expecting HAProxy to be happy, I was not expecting it to segfault.

For my use case I have an automated update system that will update configs and reload HAProxy. In between those reloads, new long-lived connections will be created from normal client usage. I came across this segfault in a test environment (not production) when trying to estimate how much reload "load" HAProxy could tolerate.

Steps to Reproduce the Behavior

  1. Run a server to accept connections (HAProxy will proxy this):

  2. Run a client in a loop that creates a long-lived connection to the server (via HAProxy), and then shortly after creating the connection, reloads HAProxy.

The below reproducer reliably crashes HAProxy after about ~250 reloads / connections.

commands:

  • haproxy (see config in other form):
 /usr/sbin/haproxy -W -f ./haproxy.cfg  -p /tmp/pid
  • server:
# socat -t 300000 - TCP-LISTEN:1213,fork,reuseaddr
  • client (reproducer):
#!/usr/bin/env python3

import time
import socket
import os

from threading import Thread

"""
create a connection to HAProxy frontend, then trigger a reload
"""

def run(i, d):
    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    s.connect(("127.0.0.1", 9090))
    d[i] = True
    print(f"cxn #{i}")
    time.sleep(9999)

def main():
    d = {}
    for i in range(2000):
        t = Thread(target=run, args=(i,d))
        t.start()
        while i not in d:
            time.sleep(.100)
        os.system("/bin/kill -USR2 $(cat /tmp/pid)")

    print('...done...')
    time.sleep(99)

main()

Do you have any idea what may have caused this?

No response

Do you have an idea how to solve the issue?

No response

What is your configuration?

frontend foo
	bind 0.0.0.0:9090
	mode tcp
	use_backend bar

backend bar
	mode tcp
	balance roundrobin
	server app 127.0.0.1:1213 weight 10

Output of haproxy -vv

HAProxy version 2.7.8-58c657f 2023/05/02 - https://haproxy.org/
Status: stable branch - will stop receiving fixes around Q1 2024.
Known bugs: http://www.haproxy.org/bugs/bugs-2.7.8.html
Running on: Linux 6.2.9-300.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Mar 30 22:32:58 UTC 2023 x86_64
Build options :
  TARGET  = linux-glibc
  CPU     = generic
  CC      = cc
  CFLAGS  = -O2 -g -Wall -Wextra -Wundef -Wdeclaration-after-statement -Wfatal-errors -Wtype-limits -Wshift-negative-value -Wshift-overflow=2 -Wduplicated-cond -Wnull-dereference -fwrapv -Wno-address-of-packed-member -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter -Wno-clobbered -Wno-missing-field-initializers -Wno-cast-function-type -Wno-string-plus-int -Wno-atomic-alignment -DMAX_SESS_STKCTR=12
  OPTIONS = USE_PCRE2=1 USE_LINUX_TPROXY=1 USE_CRYPT_H=1 USE_GETADDRINFO=1 USE_OPENSSL=1 USE_LUA=1 USE_SLZ=1 USE_SYSTEMD=1 USE_PROMEX=1
  DEBUG   = -DDEBUG_STRICT -DDEBUG_MEMORY_POOLS

Feature list : -51DEGREES +ACCEPT4 +BACKTRACE -CLOSEFROM +CPU_AFFINITY +CRYPT_H -DEVICEATLAS +DL -ENGINE +EPOLL -EVPORTS +GETADDRINFO -KQUEUE +LIBCRYPT +LINUX_SPLICE +LINUX_TPROXY +LUA -MEMORY_PROFILING +NETFILTER +NS -OBSOLETE_LINKER +OPENSSL -OPENSSL_WOLFSSL -OT -PCRE +PCRE2 -PCRE2_JIT -PCRE_JIT +POLL +PRCTL -PROCCTL +PROMEX -PTHREAD_EMULATION -QUIC +RT +SHM_OPEN +SLZ -STATIC_PCRE -STATIC_PCRE2 +SYSTEMD +TFO +THREAD +THREAD_DUMP +TPROXY -WURFL -ZLIB

Default settings :
  bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Built with multi-threading support (MAX_TGROUPS=16, MAX_THREADS=256, default=1).
Built with OpenSSL version : OpenSSL 3.0.9 30 May 2023
Running on OpenSSL version : OpenSSL 3.0.8 7 Feb 2023
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
OpenSSL providers loaded : default
Built with Lua version : Lua 5.4.4
Built with the Prometheus exporter as a service
Built with network namespace support.
Built with libslz for stateless compression.
Compression algorithms supported : identity("identity"), deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT IP_FREEBIND
Built with PCRE2 version : 10.42 2022-12-11
PCRE2 library supports JIT : no (USE_PCRE2_JIT not set)
Encrypted password support via crypt(3): yes
Built with gcc compiler version 13.1.1 20230511 (Red Hat 13.1.1-2)

Available polling systems :
      epoll : pref=300,  test result OK
       poll : pref=200,  test result OK
     select : pref=150,  test result OK
Total: 3 (3 usable), will use epoll.

Available multiplexer protocols :
(protocols marked as <default> cannot be specified using 'proto' keyword)
         h2 : mode=HTTP  side=FE|BE  mux=H2    flags=HTX|HOL_RISK|NO_UPG
       fcgi : mode=HTTP  side=BE     mux=FCGI  flags=HTX|HOL_RISK|NO_UPG
         h1 : mode=HTTP  side=FE|BE  mux=H1    flags=HTX|NO_UPG
  <default> : mode=HTTP  side=FE|BE  mux=H1    flags=HTX
       none : mode=TCP   side=FE|BE  mux=PASS  flags=NO_UPG
  <default> : mode=TCP   side=FE|BE  mux=PASS  flags=

Available services : prometheus-exporter
Available filters :
	[BWLIM] bwlim-in
	[BWLIM] bwlim-out
	[CACHE] cache
	[COMP] compression
	[FCGI] fcgi-app
	[SPOE] spoe
	[TRACE] trace

Last Outputs and Backtraces

No response

Additional Information

I've tested on 2.6, 2.7, and 2.8, and can reproduce the bug. I also tried to run as root, with no limits, and with a large maxconn.

@sigint2 sigint2 added status: needs-triage This issue needs to be triaged. type: bug This issue describes a bug. labels Jun 18, 2023
@wlallemand
Copy link
Member

Indeed this is not supposed to behave like this, which exacts versions did you test? Did you had problem like this in previous versions of HAProxy? Looks like a regression to me. I will investigate, thanks!

haproxy-mirror pushed a commit that referenced this issue Jun 19, 2023
In ticket #2184, HAProxy is crashing in a BUG_ON() after a lot of reload
when the previous processes did not exit.

Each worker has a socketpair which is a FD in the master, when reloading
this FD still exists until the process leaves. But the global.maxconn
value is not incremented for each of these FD. So when there is too much
workers and the number of FD reaches maxsock, the next FD inserted in
the poller will crash the process.

This patch fixes the issue by increasing the maxsock for each remaining
worker.

Must be backported in every maintained version.
@wlallemand
Copy link
Member

The patch above fixes the issue, however you are reaching the limit of your system with the maxsock, so instead of crashing it will fail to reload. You reach this bug because you have a low FD limit, so you should consider tweaking your environment. Maybe you are using docker or an environment with a FD limit in a unit file or something like this.

Having that much workers can lead to memory problems, so you should probably set short timeouts if you intend to reload a lot. There are also keywords like mworker-max-reloads that can help you reduce the maximum number of workers and hard-stop-after which exits the worker after a timeout even if connections are still alive.

@wlallemand wlallemand added status: fixed This issue is a now-fixed bug. 2.2 This issue affects the HAProxy 2.2 stable branch. 2.4 This issue affects the HAProxy 2.4 stable branch. 2.6 This issue affects the HAProxy 2.6 stable branch. 2.7 This issue affects the HAProxy 2.7 stable branch. 2.8 This issue affects the HAProxy 2.8 stable branch. and removed status: needs-triage This issue needs to be triaged. labels Jun 19, 2023
@sigint2
Copy link
Author

sigint2 commented Jun 20, 2023

Thank you for the quick reply and patch. I have hard-stop-after set, but I am not currently using mworker-max-reloads (I was not aware of it -- will enable it, thanks for highlighting).

As far as I can tell, the FD limit of my HAProxy master is very high (set with LimitNOFILE=... in systemd). Is there something that I could be missing with that with my HAProxy configuration? cat /proc/haproxy-master-pid/limits shows the very high number for Max open files (soft and hard limits).

@wlallemand
Copy link
Member

My mistake, you can indeed reach this problem with the default global.maxsock value computed by HAProxy, which is low in the master process.

haproxy-mirror pushed a commit that referenced this issue Jun 21, 2023
Aurelien Darragon found a case of leak when working on ticket #2184.

When a reexec_on_failure() happens *BEFORE* protocol_bind_all(), the
worker is not fork and the mworker_proc struct is still there with
its 2 socketpairs.

The socketpair that is supposed to be in the master is already closed in
mworker_cleanup_proc(), the one for the worker was suppposed to
be cleaned up in mworker_cleanlisteners().

However, since the fd is not bound during this failure, the fd is never
closed.

This patch fixes the problem by setting the fd to -1 in the mworker_proc
after the fork, so we ensure that this it won't be close if everything
was done right, and then we try to close it in mworker_cleanup_proc()
when it's not set to -1.

This could be triggered with the script in ticket #2184 and a `ulimit -H
-n 300`. This will fail before the protocol_bind_all() when trying to
increase the nofile setrlimit.

In recent version of haproxy, there is a BUG_ON() in fd_insert() that
could be triggered by this bug because of the global.maxsock check.

Must be backported as far as 2.6.

The problem could exist in previous version but the code is different
and this won't be triggered easily without other consequences in the
master.
FireBurn pushed a commit to FireBurn/haproxy that referenced this issue Jun 23, 2023
In ticket haproxy#2184, HAProxy is crashing in a BUG_ON() after a lot of reload
when the previous processes did not exit.

Each worker has a socketpair which is a FD in the master, when reloading
this FD still exists until the process leaves. But the global.maxconn
value is not incremented for each of these FD. So when there is too much
workers and the number of FD reaches maxsock, the next FD inserted in
the poller will crash the process.

This patch fixes the issue by increasing the maxsock for each remaining
worker.

Must be backported in every maintained version.

(cherry picked from commit e6051a0)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 67bddbf)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 665275e)
[wla: cfd= does not exist in 2.6]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
FireBurn pushed a commit to FireBurn/haproxy that referenced this issue Jun 23, 2023
Aurelien Darragon found a case of leak when working on ticket haproxy#2184.

When a reexec_on_failure() happens *BEFORE* protocol_bind_all(), the
worker is not fork and the mworker_proc struct is still there with
its 2 socketpairs.

The socketpair that is supposed to be in the master is already closed in
mworker_cleanup_proc(), the one for the worker was suppposed to
be cleaned up in mworker_cleanlisteners().

However, since the fd is not bound during this failure, the fd is never
closed.

This patch fixes the problem by setting the fd to -1 in the mworker_proc
after the fork, so we ensure that this it won't be close if everything
was done right, and then we try to close it in mworker_cleanup_proc()
when it's not set to -1.

This could be triggered with the script in ticket haproxy#2184 and a `ulimit -H
-n 300`. This will fail before the protocol_bind_all() when trying to
increase the nofile setrlimit.

In recent version of haproxy, there is a BUG_ON() in fd_insert() that
could be triggered by this bug because of the global.maxsock check.

Must be backported as far as 2.6.

The problem could exist in previous version but the code is different
and this won't be triggered easily without other consequences in the
master.

(cherry picked from commit 117b03f)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 8ee9a50)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 56864e3)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
FireBurn pushed a commit to FireBurn/haproxy that referenced this issue Jun 23, 2023
In ticket haproxy#2184, HAProxy is crashing in a BUG_ON() after a lot of reload
when the previous processes did not exit.

Each worker has a socketpair which is a FD in the master, when reloading
this FD still exists until the process leaves. But the global.maxconn
value is not incremented for each of these FD. So when there is too much
workers and the number of FD reaches maxsock, the next FD inserted in
the poller will crash the process.

This patch fixes the issue by increasing the maxsock for each remaining
worker.

Must be backported in every maintained version.

(cherry picked from commit e6051a0)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 67bddbf)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
FireBurn pushed a commit to FireBurn/haproxy that referenced this issue Jun 23, 2023
Aurelien Darragon found a case of leak when working on ticket haproxy#2184.

When a reexec_on_failure() happens *BEFORE* protocol_bind_all(), the
worker is not fork and the mworker_proc struct is still there with
its 2 socketpairs.

The socketpair that is supposed to be in the master is already closed in
mworker_cleanup_proc(), the one for the worker was suppposed to
be cleaned up in mworker_cleanlisteners().

However, since the fd is not bound during this failure, the fd is never
closed.

This patch fixes the problem by setting the fd to -1 in the mworker_proc
after the fork, so we ensure that this it won't be close if everything
was done right, and then we try to close it in mworker_cleanup_proc()
when it's not set to -1.

This could be triggered with the script in ticket haproxy#2184 and a `ulimit -H
-n 300`. This will fail before the protocol_bind_all() when trying to
increase the nofile setrlimit.

In recent version of haproxy, there is a BUG_ON() in fd_insert() that
could be triggered by this bug because of the global.maxsock check.

Must be backported as far as 2.6.

The problem could exist in previous version but the code is different
and this won't be triggered easily without other consequences in the
master.

(cherry picked from commit 117b03f)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 8ee9a50)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
@wlallemand wlallemand removed 2.6 This issue affects the HAProxy 2.6 stable branch. 2.7 This issue affects the HAProxy 2.7 stable branch. 2.8 This issue affects the HAProxy 2.8 stable branch. labels Jun 29, 2023
@capflam capflam removed the 2.4 This issue affects the HAProxy 2.4 stable branch. label Jul 24, 2023
FireBurn pushed a commit to FireBurn/haproxy that referenced this issue Jul 25, 2023
In ticket haproxy#2184, HAProxy is crashing in a BUG_ON() after a lot of reload
when the previous processes did not exit.

Each worker has a socketpair which is a FD in the master, when reloading
this FD still exists until the process leaves. But the global.maxconn
value is not incremented for each of these FD. So when there is too much
workers and the number of FD reaches maxsock, the next FD inserted in
the poller will crash the process.

This patch fixes the issue by increasing the maxsock for each remaining
worker.

Must be backported in every maintained version.

(cherry picked from commit e6051a0)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 67bddbf)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 665275e)
[wla: cfd= does not exist in 2.6]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 33ea988)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
FireBurn pushed a commit to FireBurn/haproxy that referenced this issue Aug 8, 2023
In ticket haproxy#2184, HAProxy is crashing in a BUG_ON() after a lot of reload
when the previous processes did not exit.

Each worker has a socketpair which is a FD in the master, when reloading
this FD still exists until the process leaves. But the global.maxconn
value is not incremented for each of these FD. So when there is too much
workers and the number of FD reaches maxsock, the next FD inserted in
the poller will crash the process.

This patch fixes the issue by increasing the maxsock for each remaining
worker.

Must be backported in every maintained version.

(cherry picked from commit e6051a0)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
FireBurn pushed a commit to FireBurn/haproxy that referenced this issue Aug 8, 2023
Aurelien Darragon found a case of leak when working on ticket haproxy#2184.

When a reexec_on_failure() happens *BEFORE* protocol_bind_all(), the
worker is not fork and the mworker_proc struct is still there with
its 2 socketpairs.

The socketpair that is supposed to be in the master is already closed in
mworker_cleanup_proc(), the one for the worker was suppposed to
be cleaned up in mworker_cleanlisteners().

However, since the fd is not bound during this failure, the fd is never
closed.

This patch fixes the problem by setting the fd to -1 in the mworker_proc
after the fork, so we ensure that this it won't be close if everything
was done right, and then we try to close it in mworker_cleanup_proc()
when it's not set to -1.

This could be triggered with the script in ticket haproxy#2184 and a `ulimit -H
-n 300`. This will fail before the protocol_bind_all() when trying to
increase the nofile setrlimit.

In recent version of haproxy, there is a BUG_ON() in fd_insert() that
could be triggered by this bug because of the global.maxsock check.

Must be backported as far as 2.6.

The problem could exist in previous version but the code is different
and this won't be triggered easily without other consequences in the
master.

(cherry picked from commit 117b03f)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
FireBurn pushed a commit to FireBurn/haproxy that referenced this issue Aug 17, 2023
In ticket haproxy#2184, HAProxy is crashing in a BUG_ON() after a lot of reload
when the previous processes did not exit.

Each worker has a socketpair which is a FD in the master, when reloading
this FD still exists until the process leaves. But the global.maxconn
value is not incremented for each of these FD. So when there is too much
workers and the number of FD reaches maxsock, the next FD inserted in
the poller will crash the process.

This patch fixes the issue by increasing the maxsock for each remaining
worker.

Must be backported in every maintained version.

(cherry picked from commit e6051a0)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 67bddbf)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 665275e)
[wla: cfd= does not exist in 2.6]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 33ea988)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 9490f9d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 7af84d7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
FireBurn pushed a commit to FireBurn/haproxy that referenced this issue Aug 17, 2023
In ticket haproxy#2184, HAProxy is crashing in a BUG_ON() after a lot of reload
when the previous processes did not exit.

Each worker has a socketpair which is a FD in the master, when reloading
this FD still exists until the process leaves. But the global.maxconn
value is not incremented for each of these FD. So when there is too much
workers and the number of FD reaches maxsock, the next FD inserted in
the poller will crash the process.

This patch fixes the issue by increasing the maxsock for each remaining
worker.

Must be backported in every maintained version.

(cherry picked from commit e6051a0)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 67bddbf)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 665275e)
[wla: cfd= does not exist in 2.6]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 33ea988)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 9490f9d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
@capflam capflam removed the 2.2 This issue affects the HAProxy 2.2 stable branch. label Aug 28, 2023
@capflam capflam closed this as completed Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: fixed This issue is a now-fixed bug. type: bug This issue describes a bug.
Projects
None yet
Development

No branches or pull requests

3 participants