BUG/MEDIUM: fcgi-app: fix memory leak in fcgi_flt_http_headers
When the loop is continued early, the memory for param_rule is not freed. This
can leak memory per request, which will eventually consume all available memory
on the server.
This patch should fix the issue #750. It must be backported as far as 2.1.
MINOR: log: adds counters on received syslog messages.
This patch adds a global counter of received syslog messages
and this one is exported on CLI "show info" as "CumRecvLogs".
This patch also updates internal conn counter and freq
of the listener and the proxy for each received log message to
prepare a further export on the "show stats".
It is possible to declare one or multiple log forwarding section,
haproxy will forward all received log messages to a log servers list.
log-forward <name>
Creates a new log forwarder proxy identified as <name>.
bind <addr> [param*]
Used to configure a log udp listener to receive messages to forward.
Only udp listeners are allowed, address must be prefixed using
'udp@', 'udp4@' or 'udp6@'. This supports for all "bind" parameters
found in 5.1 paragraph but most of them are irrelevant for udp/syslog case.
log global
log <address> [len <length>] [format <format>] [sample <ranges>:<smp_size>]
<facility> [<level> [<minlevel>]]
Used to configure target log servers. See more details on proxies
documentation.
If no format specified, haproxy tries to keep the incoming log format.
Configured facility is ignored, except if incoming message does not
present a facility but one is mandatory on the outgoing format.
If there is no timestamp available in the input format, but the field
exists in output format, haproxy will use the local date.
Example:
global
log stderr format iso local7
ring myring
description "My local buffer"
format rfc5424
maxlen 1200
size 32764
timeout connect 5s
timeout server 10s
# syslog tcp server
server mysyslogsrv 127.0.0.1:514 log-proto octet-count
log-forward sylog-loadb
bind udp4@127.0.0.1:1514
# all messages on stderr
log global
# all messages on local tcp syslog server
log ring@myring local0
# load balance messages on 4 udp syslog servers
log 127.0.0.1:10001 sample 1:4 local0
log 127.0.0.1:10002 sample 2:4 local0
log 127.0.0.1:10003 sample 3:4 local0
log 127.0.0.1:10004 sample 4:4 local0
MEDIUM: log/sink: re-work and merge of build message API.
This patch merges build message code between sink and log
and introduce a new API based on struct ist array to
prepare message header with zero copy, targeting the
log forwarding feature.
Log format 'iso' and 'timed' are now avalaible on logs line.
A new log format 'priority' is also added.
BUG/MEDIUM: log: issue mixing sampled to not sampled log servers.
A boolean was mistakenly declared 'static THREAD_LOCAL' causing
the probe of a log to a 'not sampled' log server conditionned by
the last evaluated 'sampled log' server test on the same thread.
This results to unpredictable drops of logs on 'not sampled'
log servers as soon a 'sampled' log server is declared.
This patch removes the static THREAD_LOCAL attribute from this
boolean, fixing the issue and allowing to mix 'sampled' and
'not sampled' servers.
This fix should be backported in any branches which includes
the log sampling feature.
BUG/MINOR: backend: fix potential null deref on srv_conn
Commit 08016ab82 ("MEDIUM: connection: Add private connections
synchronously in session server list") introduced a build warning about
a potential null dereference which is actually true: in case a reuse
fails an we fail to allocate a new connection, we could crash. The
issue was already present earlier but the compiler couldn't detect
it since it was guarded by an independent condition.
This should be carefully backported to older versions (at least 2.2
and maybe 2.1), the change consists in only adding a test on srv_conn.
The whole sequence of "if" blocks is ugly there and would deserve being
cleaned up so that the !srv_conn condition is matched ASAP and the
assignment is done later. This would remove complicated conditions.
BUG/MINOR: mux-fcgi: Set flags on the right stream field for empty FCGI_STDOUT
In fcgi_strm_handle_empty_stdout(), the FCGI_SF_ES_RCVD flag is set on "->state"
stream field instead of "->flags". It is obviously wrong. This bug is not
noticeable because the right state is set in the fcgi_process_demux() function a
bit later.
BUG/MINOR: mux-fcgi: Set conn state to RECORD_P when skipping the record padding
When the padding of a "stream" record (STDOUT or STDERR) is skipped, we must set
the connection state to RECORD_P. It is especially important if the padding is
not fully received.
As mentionned in the FastCGI specification, FCGI "streams" are series of
non-empty stream records (length != 0), followed by an empty one. It is properly
handled for FCGI_STDOUT records, but not for FCGI_STDERR ones. If an empty
FCGI_STDERR record is received, the connection is blocked waiting for data which
will never come.
To fix the bug, when an empty FCGI_STDERR record is received, we drop it, eating
the padding if any.
This patch should fix the issue #743. It must be backported as far as 2.1.
MINOR: raw_sock: Report the number of bytes emitted using the splicing
In the continuity of the commit 7cf0e4517 ("MINOR: raw_sock: report global
traffic statistics"), we are now able to report the global number of bytes
emitted using the splicing. It can be retrieved in "show info" output on the
CLI.
Note this counter is always declared, regardless the splicing support. This
eases the integration with monitoring tools plugged on the CLI.
BUG/MEDIUM: mux-h1: Continue to process request when switching in tunnel mode
When input data are processed, if the request is switched in tunnel mode on a
protocol upgrade, we must continue the processing. Otherwise, pending input data
will only be processed on the next wakeup. So when new input data are received,
on a timeout expiration or shutdown. Worst, if the input buffer is full when it
happens, only a timeout or a shutdown will unblock the situation.
This patch should fix the issue #737. It must be backported as far as 1.9. The
bug does not seem to affect the 2.0 and 1.9 because, on a protocol upgrade, the
request is switched in tunnel mode when the response is sent to the client. But
the bug is present, so the backport remains necessary.
MINOR: server: Factorize code to deal with connections removed from an idle list
The srv_del_conn_from_list() function is now responsible to update the server
counters and the connection flags when a connection is removed from an idle list
(safe, idle or available). It is called when a connection is released or when a
connection is set as private. This function also removes the connection from the
idle list if necessary.
MINOR: server: Factorize code to deal with reuse of server idle connections
The srv_use_idle_conn() function is now responsible to update the server
counters and the connection flags when an idle connection is reused. The same
function is called when a new connection is created. This simplifies a bit the
connect_server() function.
MINOR: connection: Set the conncetion target during its initialisation
When a new connection is created, its target is always set just after. So the
connection target may set when it is created instead, during its initialisation
to be precise. It is the purpose of this patch. Now, conn_new() function is
called with the connection target as parameter. The target is then passed to
conn_init(). It means the target must be passed when cs_new() is called. In this
case, the target is only used when the conn-stream is created with no
connection. This only happens for tcpchecks for now.
MINOR: connection: Use a dedicated function to look for a session's connection
The session_get_conn() must now be used to look for an available connection
matching a specific target for a given session. This simplifies a bit the
connect_server() function.
MEDIUM: connection: Add private connections synchronously in session server list
When a connection is marked as private, it is now added in the session server
list. We don't wait a stream is detached from the mux to do so. When the
connection is created, this happens after the mux creation. Otherwise, it is
performed when the connection is marked as private.
To allow that, when a connection is created, the session is systematically set
as the connectin owner. Thus, a backend connection has always a owner during its
creation. And a private connection has always a owner until its death.
Note that outside the detach() callback, if the call to session_add_conn()
failed, the error is ignored. In this situation, we retry to add the connection
into the session server list in the detach() callback. If this fails at this
step, the multiplexer is destroyed and the connection is closed.
MINOR: connection: Add a wrapper to mark a connection as private
To set a connection as private, the conn_set_private() function must now be
called. It sets the CO_FL_PRIVATE flags, but it also remove the connection from
the available connection list, if necessary. For now, it never happens because
only HTTP/1 connections may be set as private after their creation. And these
connections are never inserted in the available connection list.
MINOR: connection: Set new connection as private on reuse never
When a new connection is created, it may immediatly be set as private if
http-reuse never is configured for the backend. There is no reason to wait the
call to mux->detach() to do so.
MINOR: connection: Set the SNI on server connections before installing the mux
If an expression is configured to set the SNI on a server connection, the
connection is marked as private. To not needlessly add it in the available
connection list when the mux is installed, the SNI is now set on the connection
before installing the mux, just after the call to si_connect().
BUG/MEDIUM: mux-fcgi: Don't add private connections in available connection list
When a stream is detached from a backend private connection, we must not insert
it in the available connection list. In addition, we must be sure to remove it
from this list. To ensure it is properly performed, this part has been slightly
refactored to clearly split processing of private connections from the others.
BUG/MEDIUM: mux-h2: Don't add private connections in available connection list
When a stream is detached from a backend private connection, we must not insert
it in the available connection list. In addition, we must be sure to remove it
from this list. To ensure it is properly performed, this part has been slightly
refactored to clearly split processing of private connections from the others.
CI: travis-ci: speed up osx build by running brew scripted, switch to latest osx image
we used to use travis-ci brew plugin to install "socat", travis-ci brew
plugin works predictable in "all update" mode. sometimes it might take 12 minutes.
let us improve developer velocity by running brew from command line. It takes 2 minutes
instead of 12 minutes
latest osx seems to have more stable brew, let us switch to latest osx available.
osx images list: https://docs.travis-ci.com/user/reference/osx/#macos-version
CONTRIB: da: fix memory leak in dummy function da_atlas_open()
The dummy function takes care of doing a bit of work using a malloc()
to avoid returning a constant but it doesn't free the tested pointer,
which coverity noticed in issue #741. Let's free it before testing it
for the return value.
This may be backported but is not important since this code is only
present to allow to build the device detection code and not to actually
run it.
MINOR: tasks: use MT_LIST_ADDQ() when killing tasks.
A bug in task_kill() was fixed by commy 54d31170a ("BUG/MAJOR: sched:
make sure task_kill() always queues the task") which added a list
initialization before adding an element. But in fact an inconditional
addition would have done the same and been simpler than first
initializing then checking the element was initialized. Let's use
MT_LIST_ADDQ() there to add the task to kill into the shared queue
and kill the dirty LIST_INIT().
MINOR: connection: use MT_LIST_ADDQ() to add connections to idle lists
When a connection is added to an idle list, it's already detached and
cannot be seen by two threads at once, so there's no point using
TRY_ADDQ, there will never be any conflict. Let's just use the cheaper
ADDQ.
MINOR: buffer: use MT_LIST_ADDQ() for buffer_wait lists additions
The TRY_ADDQ there was not needed since the wait list is exclusively
owned by the caller. There's a preliminary test on MT_LIST_ADDED()
that might have been eliminated by keeping MT_LIST_TRY_ADDQ() but
it would have required two more expensive writes before testing so
better keep the test the way it is.
MINOR: lists: rename some MT_LIST operations to clarify them
Initially when mt_lists were added, their purpose was to be used with
the scheduler, where anyone may concurrently add the same tasklet, so
it sounded natural to implement a check in MT_LIST_ADD{,Q}. Later their
usage was extended and MT_LIST_ADD{,Q} started to be used on situations
where the element to be added was exclusively owned by the one performing
the operation so a conflict was impossible. This became more obvious with
the idle connections and the new macro was called MT_LIST_ADDQ_NOCHECK.
But this remains confusing and at many places it's not expected that
an MT_LIST_ADD could possibly fail, and worse, at some places we start
by initializing it before adding (and the test is superflous) so let's
rename them to something more conventional to denote the presence of the
check or not:
MT_LIST_ADD{,Q} : inconditional operation, the caller owns the
element, and doesn't care about the element's
current state (exactly like LIST_ADD)
MT_LIST_TRY_ADD{,Q}: only perform the operation if the element is not
already added or in the process of being added.
This means that the previously "safe" MT_LIST_ADD{,Q} are not "safe"
anymore. This also means that in case of backport mistakes in the
future causing this to be overlooked, the slower and safer functions
will still be used by default.
Note that the missing unchecked MT_LIST_ADD macro was added.
The rest of the code will have to be reviewed so that a number of
callers of MT_LIST_TRY_ADDQ are changed to MT_LIST_ADDQ to remove
the unneeded test.
BUILD: tcp: condition TCP keepalive settings to platforms providing them
Previous commit b24bc0d ("MINOR: tcp: Support TCP keepalive parameters
customization") broke non-Linux builds as TCP_KEEP{CNT,IDLE,INTVL} are
not necessarily defined elsewhere.
This patch adds the required #ifdefs to condition the visibility of the
keywords, and adds a mention in the doc about their dependency on Linux.
MINOR: tcp: Support TCP keepalive parameters customization
It is now possible to customize TCP keepalive parameters.
These correspond to the socket options TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
and are valid for the defaults, listen, frontend and backend sections.
BUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ
The torture test run for previous commit 787dc20 ("BUG/MEDIUM: lists: add
missing store barrier on MT_LIST_BEHEAD()") finally broke again after 34M
connections. It appeared that MT_LIST_ADD and MT_LIST_ADDQ were suffering
from the same missing barrier when restoring the original pointers before
giving up, when checking if the element was already added. This is indeed
something which seldom happens with the shared scheduler, in case two
threads simultaneously try to wake up the same tasklet.
With a store barrier there after reverting the pointers, the torture test
survived 750M connections on the NanoPI-Fire3, so it looks good this time.
Probably that MT_LIST_BEHEAD should be added to test-list.c since it seems
to be more sensitive to concurrent accesses with MT_LIST_ADDQ.
It's worth noting that there is no barrier between the last two pointers
update, while there is one in MT_LIST_POP and MT_LIST_BEHEAD, the latter
having shown to be needed, but I cannot demonstrate why we would need
one here. Given that the code seems solid here, let's stick to what is
shown to work.
This fix should be backported to 2.1, just for the sake of safety since
the issue couldn't be triggered there, but it could change with the
compiler or when backporting a fix for example.
BUG/MEDIUM: lists: add missing store barrier on MT_LIST_BEHEAD()
When running multi-threaded tests on my NanoPI-Fire3 (8 A53 cores), I
managed to occasionally get either a bus error or a segfault in the
scheduler, but only when running at a high connection rate (injecting
on a tcp-request connection reject rule). The bug is rare and happens
around once per million connections. I could never reproduce it with
less than 4 threads nor on A72 cores.
Haproxy 2.1.0 would also fail there but not 2.1.7.
Every time the crash happened with the TL_URGENT task list corrupted,
though it was not immediately after the LIST_SPLICE() call, indicating
background activity survived the MT_LIST_BEHEAD() operation. This queue
is where the shared runqueue is transferred, and the shared runqueue
gets fast inter-thread tasklet wakeups from idle conn takeover and new
connections.
Comparing the MT_LIST_BEHEAD() and MT_LIST_DEL() implementations, it's
quite obvious that a few barriers are missing from the former, and these
will simply fail on weakly ordered caches.
Two store barriers were added before the break() on failure, to match
what is done on the normal path. Missing them almost always results in
a segfault which is quite rare but consistent (after ~3M connections).
The 3rd one before updating n->prev seems intuitively needed though I
coudln't make the code fail without it. It's present in MT_LIST_DEL so
better not be needlessly creative. The last one is the most important
one, and seems to be the one that helps a concurrent MT_LIST_ADDQ()
detect a late failure and try again. With this, the code survives at
least 30M connections.
Interestingly the exact same issue was addressed in 2.0-dev2 for MT_LIST_DEL
with commit 690d2ad4d ("BUG/MEDIUM: list: add missing store barriers when
updating elements and head").
This fix must be backported to 2.1 as MT_LIST_BEHEAD() is also used there.
It's only tagged as medium because it will only affect entry-level CPUs
like Cortex A53 (x86 are not affected), and requires load levels that are
very hard to achieve on such machines to trigger it. In practice it's
unlikely anyone will ever hit it.
Tim Duesterhus [Sat, 4 Jul 2020 09:53:26 +0000 (11:53 +0200)]
CLEANUP: Add static void hlua_deinit()
Compiling HAProxy with USE_LUA=1 and running a configuration check within
valgrind with a very simple configuration such as:
listen foo
bind *:8080
Will report quite a few possible leaks afterwards:
==24048== LEAK SUMMARY:
==24048== definitely lost: 0 bytes in 0 blocks
==24048== indirectly lost: 0 bytes in 0 blocks
==24048== possibly lost: 95,513 bytes in 1,209 blocks
==24048== still reachable: 329,960 bytes in 71 blocks
==24048== suppressed: 0 bytes in 0 blocks
Printing these possible leaks shows that all of them are caused by Lua.
Luckily Lua makes it *very* easy to free all used memory, so let's do
this on shutdown.
Afterwards this patch is applied the output looks much better:
==24199== LEAK SUMMARY:
==24199== definitely lost: 0 bytes in 0 blocks
==24199== indirectly lost: 0 bytes in 0 blocks
==24199== possibly lost: 0 bytes in 0 blocks
==24199== still reachable: 329,960 bytes in 71 blocks
==24199== suppressed: 0 bytes in 0 blocks
Tim Duesterhus [Sat, 4 Jul 2020 09:49:46 +0000 (11:49 +0200)]
BUG/MINOR: sample: Free str.area in smp_check_const_meth
Given the following example configuration:
listen foo
mode http
bind *:8080
http-request set-var(txn.leak) meth(GET)
server x example.com:80
Running a configuration check with valgrind reports:
==25992== 4 bytes in 1 blocks are definitely lost in loss record 1 of 344
==25992== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25992== by 0x4E239D: my_strndup (tools.c:2261)
==25992== by 0x581E20: make_arg_list (arg.c:253)
==25992== by 0x4DE91D: sample_parse_expr (sample.c:890)
==25992== by 0x58E304: parse_store (vars.c:772)
==25992== by 0x566A3F: parse_http_req_cond (http_rules.c:95)
==25992== by 0x4A4CE6: cfg_parse_listen (cfgparse-listen.c:1339)
==25992== by 0x494C59: readcfgfile (cfgparse.c:2049)
==25992== by 0x545145: init (haproxy.c:2029)
==25992== by 0x421E42: main (haproxy.c:3175)
After this patch is applied the leak is gone as expected.
This is a fairly minor leak, but it can add up for many uses of the `bool()`
sample fetch. The bug most likely exists since the `bool()` sample fetch was
introduced in commit cc103299c75c530ab3637a1698306145bdc85552. The fix may
be backported to HAProxy 1.6+.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:45 +0000 (11:49 +0200)]
BUG/MINOR: sample: Free str.area in smp_check_const_bool
Given the following example configuration:
listen foo
mode http
bind *:8080
http-request set-var(txn.leak) bool(1)
server x example.com:80
Running a configuration check with valgrind reports:
==24233== 2 bytes in 1 blocks are definitely lost in loss record 1 of 345
==24233== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24233== by 0x4E238D: my_strndup (tools.c:2261)
==24233== by 0x581E10: make_arg_list (arg.c:253)
==24233== by 0x4DE90D: sample_parse_expr (sample.c:890)
==24233== by 0x58E2F4: parse_store (vars.c:772)
==24233== by 0x566A2F: parse_http_req_cond (http_rules.c:95)
==24233== by 0x4A4CE6: cfg_parse_listen (cfgparse-listen.c:1339)
==24233== by 0x494C59: readcfgfile (cfgparse.c:2049)
==24233== by 0x545135: init (haproxy.c:2029)
==24233== by 0x421E42: main (haproxy.c:3175)
After this patch is applied the leak is gone as expected.
This is a fairly minor leak, but it can add up for many uses of the `bool()`
sample fetch. The bug most likely exists since the `bool()` sample fetch was
introduced in commit cc103299c75c530ab3637a1698306145bdc85552. The fix may
be backported to HAProxy 1.6+.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:44 +0000 (11:49 +0200)]
BUG/MINOR: haproxy: Free srule->expr during deinit
Given the following example configuration:
backend foo
mode http
use-server %[str(x)] if { always_true }
server x example.com:80
Running a configuration check with valgrind reports:
==19376== 170 (40 direct, 130 indirect) bytes in 1 blocks are definitely lost in loss record 281 of 347
==19376== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19376== by 0x5091AC: add_sample_to_logformat_list (log.c:511)
==19376== by 0x50A5A6: parse_logformat_string (log.c:671)
==19376== by 0x4957F2: check_config_validity (cfgparse.c:2588)
==19376== by 0x54442D: init (haproxy.c:2129)
==19376== by 0x421E42: main (haproxy.c:3169)
After this patch is applied the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:43 +0000 (11:49 +0200)]
BUG/MINOR: haproxy: Free srule->file during deinit
Given the following example configuration:
backend foo
mode http
use-server x if { always_true }
server x example.com:80
Running a configuration check with valgrind reports:
==18650== 14 bytes in 1 blocks are definitely lost in loss record 3 of 345
==18650== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18650== by 0x649E489: strdup (strdup.c:42)
==18650== by 0x4A5438: cfg_parse_listen (cfgparse-listen.c:1548)
==18650== by 0x494C59: readcfgfile (cfgparse.c:2049)
==18650== by 0x5450B5: init (haproxy.c:2029)
==18650== by 0x421E42: main (haproxy.c:3168)
After this patch is applied the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:42 +0000 (11:49 +0200)]
BUG/MINOR: haproxy: Free proxy->unique_id_header during deinit
Given the following example configuration:
frontend foo
mode http
bind *:8080
unique-id-header x
Running a configuration check with valgrind reports:
==17621== 2 bytes in 1 blocks are definitely lost in loss record 1 of 341
==17621== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17621== by 0x649E489: strdup (strdup.c:42)
==17621== by 0x4A87F1: cfg_parse_listen (cfgparse-listen.c:2747)
==17621== by 0x494C59: readcfgfile (cfgparse.c:2049)
==17621== by 0x545095: init (haproxy.c:2029)
==17621== by 0x421E42: main (haproxy.c:3167)
After this patch is applied the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:41 +0000 (11:49 +0200)]
BUG/MINOR: haproxy: Add missing free of server->(hostname|resolvers_id)
Given the following example configuration:
resolvers test
nameserver test 127.0.0.1:53
listen foo
bind *:8080
server foo example.com resolvers test
Running a configuration check within valgrind reports:
==21995== 5 bytes in 1 blocks are definitely lost in loss record 1 of 30
==21995== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21995== by 0x5726489: strdup (strdup.c:42)
==21995== by 0x4B2CFB: parse_server (server.c:2163)
==21995== by 0x4680C1: cfg_parse_listen (cfgparse-listen.c:534)
==21995== by 0x459E33: readcfgfile (cfgparse.c:2167)
==21995== by 0x50778D: init (haproxy.c:2021)
==21995== by 0x418262: main (haproxy.c:3133)
==21995==
==21995== 12 bytes in 1 blocks are definitely lost in loss record 3 of 30
==21995== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21995== by 0x5726489: strdup (strdup.c:42)
==21995== by 0x4AC666: srv_prepare_for_resolution (server.c:1606)
==21995== by 0x4B2EBD: parse_server (server.c:2081)
==21995== by 0x4680C1: cfg_parse_listen (cfgparse-listen.c:534)
==21995== by 0x459E33: readcfgfile (cfgparse.c:2167)
==21995== by 0x50778D: init (haproxy.c:2021)
==21995== by 0x418262: main (haproxy.c:3133)
with one more leak unrelated to `struct server`. After applying this
patch the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
Tim Duesterhus [Sat, 4 Jul 2020 09:49:40 +0000 (11:49 +0200)]
BUG/MINOR: haproxy: Free proxy->format_unique_id during deinit
Given the following example configuration:
frontend foo
mode http
bind *:8080
unique-id-format x
Running a configuration check with valgrind reports:
==30712== 42 (40 direct, 2 indirect) bytes in 1 blocks are definitely lost in loss record 18 of 39
==30712== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30712== by 0x4ED7E9: add_to_logformat_list (log.c:462)
==30712== by 0x4EEE28: parse_logformat_string (log.c:720)
==30712== by 0x47B09A: check_config_validity (cfgparse.c:3046)
==30712== by 0x52881D: init (haproxy.c:2121)
==30712== by 0x41F382: main (haproxy.c:3126)
After this patch is applied the leak is gone as expected.
This is a very minor leak that can only be observed if deinit() is called,
shortly before the OS will free all memory of the process anyway. No
backport needed.
Running a configuration check within valgrind reports:
==1431== 32 bytes in 1 blocks are definitely lost in loss record 20 of 43
==1431== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1431== by 0x4C39B5: sample_parse_expr (sample.c:982)
==1431== by 0x56B410: parse_acl_expr (acl.c:319)
==1431== by 0x56BA7F: parse_acl (acl.c:697)
==1431== by 0x48D225: cfg_parse_listen (cfgparse-listen.c:816)
==1431== by 0x4797C3: readcfgfile (cfgparse.c:2167)
==1431== by 0x52943D: init (haproxy.c:2021)
==1431== by 0x41F382: main (haproxy.c:3133)
After this patch is applied the leak is gone as expected.
This is a fairly minor leak that can only be observed if samples need to be
freed, which is not something that should occur during normal processing and
most likely only during shut down. Thus no backport should be needed.
Running a configuration check within valgrind reports:
==31371== 160 (48 direct, 112 indirect) bytes in 1 blocks are definitely lost in loss record 35 of 45
==31371== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31371== by 0x4C3832: sample_parse_expr (sample.c:876)
==31371== by 0x56B3E0: parse_acl_expr (acl.c:319)
==31371== by 0x56BA4F: parse_acl (acl.c:697)
==31371== by 0x48D225: cfg_parse_listen (cfgparse-listen.c:816)
==31371== by 0x4797C3: readcfgfile (cfgparse.c:2167)
==31371== by 0x5293ED: init (haproxy.c:2021)
==31371== by 0x41F382: main (haproxy.c:3126)
After this patch this leak is reduced. It will be fully removed in a
follow up patch:
==32503== 32 bytes in 1 blocks are definitely lost in loss record 20 of 43
==32503== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==32503== by 0x4C39B5: sample_parse_expr (sample.c:982)
==32503== by 0x56B410: parse_acl_expr (acl.c:319)
==32503== by 0x56BA7F: parse_acl (acl.c:697)
==32503== by 0x48D225: cfg_parse_listen (cfgparse-listen.c:816)
==32503== by 0x4797C3: readcfgfile (cfgparse.c:2167)
==32503== by 0x52943D: init (haproxy.c:2021)
==32503== by 0x41F382: main (haproxy.c:3133)
This is a fairly minor leak that can only be observed if ACLs need to be
freed, which is not something that should occur during normal processing
and most likely only during shut down. Thus no backport should be needed.
Released version 2.3-dev0 with the following main changes :
- [RELEASE] Released version 2.3-dev0
- MINOR: version: back to development, update status message
Released version 2.2.0 with the following main changes :
- BUILD: mux-h2: fix typo breaking build when using DEBUG_LOCK
- CLEANUP: makefile: update the outdated list of DEBUG_xxx options
- BUILD: tools: make resolve_sym_name() return a const
- CLEANUP: auth: fix useless self-include of auth-t.h
- BUILD: tree-wide: cast arguments to tolower/toupper to unsigned char
- CLEANUP: assorted typo fixes in the code and comments
- WIP/MINOR: ssl: add sample fetches for keylog in frontend
- DOC: fix tune.ssl.keylog sample fetches array
- BUG/MINOR: ssl: check conn in keylog sample fetch
- DOC: configuration: various typo fixes
- MINOR: log: Remove unused case statement during the log-format string parsing
- BUG/MINOR: mux-h1: Fix the splicing in TUNNEL mode
- BUG/MINOR: mux-h1: Don't read data from a pipe if the mux is unable to receive
- BUG/MINOR: mux-h1: Disable splicing only if input data was processed
- BUG/MEDIUM: mux-h1: Disable splicing for the conn-stream if read0 is received
- MINOR: mux-h1: Improve traces about the splicing
- BUG/MINOR: backend: Remove CO_FL_SESS_IDLE if a client remains on the last server
- BUG/MEDIUM: connection: Don't consider new private connections as available
- BUG/MINOR: connection: See new connection as available only on reuse always
- DOC: configuration: remove obsolete mentions of H2 being converted to HTTP/1.x
- CLEANUP: ssl: remove unrelevant comment in smp_fetch_ssl_x_keylog()
- DOC: update INSTALL with new compiler versions
- DOC: minor update to coding style file
- MINOR: version: mention that it's an LTS release now
DOC: configuration: remove obsolete mentions of H2 being converted to HTTP/1.x
The first H2 implementation in version 1.8 used to turn HTTP/2 requests
to HTTP/1.1, causing many limitations. This is not true anymore and we
don't suffer from the lack of server-side H2 nor are we forced to close
mode anymore, so let's remove such obsolete mentions.
BUG/MINOR: connection: See new connection as available only on reuse always
When the multiplexer creation is delayed after the handshakes phase, the
connection is added in the available connection list if http-reuse never is not
configured for the backend. But it is a wrong statement. At this step, the
connection is not safe because it is a new connection. So it must be added in
the available connection list only if http-reuse always is used.
BUG/MEDIUM: connection: Don't consider new private connections as available
When a connection is created and the multiplexer is installed, if the connection
is marked as private, don't consider it as available, regardless the number of
available streams. This test is performed when the mux is installed when the
connection is created, in connect_server(), and when the mux is installed after
the handshakes stage.
BUG/MINOR: backend: Remove CO_FL_SESS_IDLE if a client remains on the last server
When a connection is picked from the session server list because the proxy or
the session are marked to use the last requested server, if it is idle, we must
marked it as used removing the CO_FL_SESS_IDLE flag and decrementing the session
idle_conns counter.
BUG/MEDIUM: mux-h1: Disable splicing for the conn-stream if read0 is received
The CS_FL_MAY_SPLICE flag must be unset for the conn-stream if a read0 is
received while reading on the kernel pipe. It is mandatory when some data was
also received. Otherwise, this flag prevent the call to the h1 rcv_buf()
callback. Thus the read0 will never be handled by the h1 multiplexer leading to
a freeze of the session until a timeout is reached.
BUG/MINOR: mux-h1: Disable splicing only if input data was processed
In h1_rcv_buf(), the splicing is systematically disabled if it was previously
enabled. When it happens, if the splicing is enabled it means the channel's
buffer was empty before calling h1_rcv_buf(). Thus, the only reason to disable
the splicing at this step is when some input data have just been processed.
BUG/MINOR: mux-h1: Don't read data from a pipe if the mux is unable to receive
In h1_rcv_pipe(), if the mux is unable to receive data, for instance because the
multiplexer is blocked on input waiting the other side (BUSY mode), no receive
must be performed.
BUG/MINOR: mux-h1: Fix the splicing in TUNNEL mode
In the commit 17ccd1a35 ("BUG/MEDIUM: connection: add a mux flag to indicate
splice usability"), The CS_FL_MAY_SPLICE flags was added to notify the upper
layer that the mux is able to use the splicing. But this was only done for the
payload in a message, in HTTP_MSG_DATA state. But the splicing is also possible
in TUNNEL mode, in HTTP_MSG_TUNNEL state. In addition, the splicing ability is
always disabled for chunked messages.
MINOR: log: Remove unused case statement during the log-format string parsing
Since the commit cd0d2ed6e ("MEDIUM: log-format: make the LF parser aware of
sample expressions' end"), the LF_STEXPR label in the last switch-case statement
at the end of the for loop in the parse_logformat_string() function cannot be
reached anymore.
WIP/MINOR: ssl: add sample fetches for keylog in frontend
OpenSSL 1.1.1 provides a callback registering function
SSL_CTX_set_keylog_callback, which allows one to receive a string
containing the keys to deciphers TLSv1.3.
Unfortunately it is not possible to store this data in binary form and
we can only get this information using the callback. Which means that we
need to store it until the connection is closed.
This patches add 2 pools, the first one, pool_head_ssl_keylog is used to
store a struct ssl_keylog which will be inserted as a ex_data in a SSL *.
The second one is pool_head_ssl_keylog_str which will be used to store
the hexadecimal strings.
To enable the capture of the keys, you need to set "tune.ssl.keylog on"
in your configuration.
BUILD: tree-wide: cast arguments to tolower/toupper to unsigned char
NetBSD apparently uses macros for tolower/toupper and complains about
the use of char for array subscripts. Let's properly cast all of them
to unsigned char where they are used.
BUILD: tools: make resolve_sym_name() return a const
Originally it was made to return a void* because some comparisons in the
code where it was used required a lot of casts. But now we don't need
that anymore. And having it non-const breaks the build on NetBSD 9 as
reported in issue #728.
So let's switch to const and adjust debug.c to accomodate this.
CLEANUP: makefile: update the outdated list of DEBUG_xxx options
A few options didn't exist anymore (FSM, HASH) and quite a few ones were
added since last update (MEM_STATS, DONT_SHARE_POOLS, NO_LOCKLESS_POOLS,
NO_LOCAL_POOLS, FAIL_ALLOC, STRICT_NOCRASH, HPACK.
BUILD: mux-h2: fix typo breaking build when using DEBUG_LOCK
A typo was accidently introduced in commit 48ce6a3 ("BUG/MEDIUM: muxes:
Make sure nobody stole the connection before using it."), a "&" was
placed in front of "OTHER_LOCK", which breaks DEBUG_LOCK. No backport
is needed.
Released version 2.2-dev12 with the following main changes :
- BUG/MINOR: mux_h2: don't lose the leaving trace in h2_io_cb()
- MINOR: cli: make "show sess" stop at the last known session
- CLEANUP: buffers: remove unused buffer_wq_lock lock
- BUG/MEDIUM: buffers: always allocate from the local cache first
- MINOR: connection: align toremove_{lock,connections} and cleanup into idle_conns
- CONTRIB: debug: add missing flags SI_FL_L7_RETRY & SI_FL_D_L7_RETRY
- BUG/MEDIUM: connections: Don't increase curr_used_conns for shared connections.
- BUG/MEDIUM: checks: Increment the server's curr_used_conns
- REORG: buffer: rename buffer.c to dynbuf.c
- REORG: includes: create tinfo.h for the thread_info struct
- CLEANUP: pool: only include the type files from types
- MINOR: pools: move the LRU cache heads to thread_info
- BUG/MINOR: debug: fix "show fd" null-deref when built with DEBUG_FD
- MINOR: stats: add 3 new output values for the per-server idle conn state
- MINOR: activity: add per-thread statistics on FD takeover
- BUG/MINOR: server: start cleaning idle connections from various points
- MEDIUM: server: improve estimate of the need for idle connections
- MINOR: stats: add the estimated need of concurrent connections per server
- BUG/MINOR: threads: Don't forget to init each thread toremove_lock.
- BUG/MEDIUM: lists: Lock the element while we check if it is in a list.
- Revert "BUG/MEDIUM: lists: Lock the element while we check if it is in a list."
- BUG/MINOR: haproxy: don't wake already stopping threads on exit
- BUG/MINOR: server: always count one idle slot for current thread
- MEDIUM: server: use the two thresholds for the connection release algorithm
- BUG/MINOR: http-rules: Fix ACLs parsing for http deny rules
- BUG/MINOR: sched: properly cover for a rare MT_LIST_ADDQ() race
- MINOR: mux-h1: avoid taking the toremove_lock in on dying tasks
- MINOR: mux-h2: avoid taking the toremove_lock in on dying tasks
- MINOR: mux-fcgi: avoid taking the toremove_lock in on dying tasks
- MINOR: pools: increase MAX_BASE_POOLS to 64
- DOC: ssl: add "allow-0rtt" and "ciphersuites" in crt-list
- BUG/MEDIUM: pattern: Add a trailing \0 to match strings only if possible
- BUG/MEDIUM: log-format: fix possible endless loop in parse_logformat_string()
- BUG/MINOR: proxy: fix dump_server_state()'s misuse of the trash
- BUG/MINOR: proxy: always initialize the trash in show servers state
- MINOR: cli/proxy: add a new "show servers conn" command
- MINOR: server: skip servers with no idle conns earlier
- BUG/MINOR: server: fix the connection release logic regarding nearly full conditions
- MEDIUM: server: add a new pool-low-conn server setting
- BUG/MEDIUM: backend: always search in the safe list after failing on the idle one
- MINOR: backend: don't always takeover from the same threads
- MINOR: sched: make sched->task_list_size atomic
- MEDIUM: sched: create a new TASK_KILLED task flag
- MEDIUM: sched: implement task_kill() to kill a task
- MEDIUM: mux-h1: use task_kill() during h1_takeover() instead of task_wakeup()
- MEDIUM: mux-h2: use task_kill() during h2_takeover() instead of task_wakeup()
- MEDIUM: mux-fcgi: use task_kill() during fcgi_takeover() instead of task_wakeup()
- MINOR: list: Add MT_LIST_DEL_SAFE_NOINIT() and MT_LIST_ADDQ_NOCHECK()
- CLEANUP: connections: rename the toremove_lock to takeover_lock
- MEDIUM: connections: Don't use a lock when moving connections to remove.
- DOC: configuration: add missing index entries for tune.pool-{low,high}-fd-ratio
- DOC: configuration: fix alphabetical ordering for tune.pool-{high,low}-fd-ratio
- MINOR: config: add a new tune.idle-pool.shared global setting.
- MINOR: 51d: silence a warning about null pointer dereference
- MINOR: debug: add a new "debug dev memstats" command
- MINOR: log-format: allow to preserve spacing in log format strings
- BUILD: debug: avoid build warnings with DEBUG_MEM_STATS
- BUG/MAJOR: sched: make sure task_kill() always queues the task
- BUG/MEDIUM: muxes: Make sure nobody stole the connection before using it.
- BUG/MEDIUM: cli/proxy: don't try to dump idle connection state if there's none
- BUILD: haproxy: fix build error when RLIMIT_AS is not set
- BUG/MAJOR: sched: make it work also when not building with DEBUG_STRICT
- MINOR: log: add time second fraction field to rfc5424 log timestamp.
- BUG/MINOR: log: missing timezone on iso dates.
- BUG/MEDIUM: server: don't kill all idle conns when there are not enough
- MINOR: sched: split tasklet_wakeup() into tasklet_wakeup_on()
- BUG/MEDIUM: connections: Set the tid for the old tasklet on takeover.
- BUG/MEDIUM: connections: Let the xprt layer know a takeover happened.
- BUG/MINOR: http_act: don't check capture id in backend (2)
- BUILD: makefile: disable threads by default on OpenBSD
- BUILD: peers: fix build warning with gcc 4.2.1
- CI: cirrus-ci: exclude slow reg-tests
Building on OpenBSD 6.7 with gcc-4.2.1 yields the following warnings
which suggest that the initialization is not taken as expected but
that the container member is reset with each initialization:
src/peers.c: In function 'peer_send_updatemsg':
src/peers.c:1000: warning: initialized field overwritten
src/peers.c:1000: warning: (near initialization for 'p.updt')
src/peers.c:1001: warning: initialized field overwritten
src/peers.c:1001: warning: (near initialization for 'p.updt')
src/peers.c:1002: warning: initialized field overwritten
src/peers.c:1002: warning: (near initialization for 'p.updt')
src/peers.c:1003: warning: initialized field overwritten
src/peers.c:1003: warning: (near initialization for 'p.updt')
src/peers.c:1004: warning: initialized field overwritten
src/peers.c:1004: warning: (near initialization for 'p.updt')
Fixing this is trivial, we just have to initialize one level at
a time.
BUILD: makefile: disable threads by default on OpenBSD
As reported by Ilya in issue #725, building with threads on OpenBSD
is broken with gcc:
include/haproxy/tinfo.h:30: error: thread-local storage not supported for this target
Better stay safe and disable it. Clang seems to support (or emulate)
thread-local, at least it builds. Those willing to experiment can
easily pass USE_THREAD=1.
Tim Duesterhus [Fri, 3 Jul 2020 11:43:42 +0000 (13:43 +0200)]
BUG/MINOR: http_act: don't check capture id in backend (2)
Please refer to commit 19a69b3740702ce5503a063e9dfbcea5b9187d27 for all the
details. This follow up commit fixes the `http-response capture` case, the
previous one only fixed the `http-request capture` one. The documentation was
already updated and the change to `check_http_res_capture` is identical to
the `check_http_req_capture` change.
BUG/MEDIUM: connections: Let the xprt layer know a takeover happened.
When we takeover a connection, let the xprt layer know. If it has its own
tasklet, and it is already scheduled, then it has to be destroyed, otherwise
it may run the new mux tasklet on the old thread.
Note that we only do this for the ssl xprt for now, because the only other
one that might wake the mux up is the handshake one, which is supposed to
disappear before idle connections exist.
BUG/MEDIUM: connections: Set the tid for the old tasklet on takeover.
In the various takeover() methods, make sure we schedule the old tasklet
on the old thread, as we don't want it to run on our own thread! This
was causing a very rare crash when building with DEBUG_STRICT, seeing
that either an FD's thread mask didn't match the thread ID in h1_io_cb(),
or that stream_int_notify() would try to queue a task with the wrong
tid_bit.
In order to reproduce this, it is necessary to maintain many connections
(typically 30k) at a high request rate flowing over H1+SSL between two
proxies, the second of which would randomly reject ~1% of the incoming
connection and randomly killing some idle ones using a very short client
timeout. The request rate must be adjusted so that the CPUs are nearly
saturated, but never reach 100%. It's easier to reproduce this by skipping
local connections and always picking from other threads. The issue
should happen in less than 20s otherwise it's necessary to restart to
reset the idle connections lists.
MINOR: sched: split tasklet_wakeup() into tasklet_wakeup_on()
tasklet_wakeup() only checks tl->tid to know whether the task is
programmed to run on the current thread or on a specific thread. We'll
have to ease this selection in a subsequent patch, preferably without
modifying tl->tid, so let's have a new tasklet_wakeup_on() function
to specify the thread number to run on. That the logic has not changed
at all.
BUG/MEDIUM: server: don't kill all idle conns when there are not enough
In srv_cleanup_idle_connections(), we compute how many idle connections
are in excess compared to the average need. But we may actually be missing
some, for example if a certain number were recently closed and the average
of used connections didn't change much since previous period. In this
case exceed_conn can become negative. There was no special case for this
in the code, and calculating the per-thread share of connections to kill
based on this value resulted in special value -1 to be passed to
srv_migrate_conns_to_remove(), which for this function means "kill all of
them", as used in srv_cleanup_connections() for example.
This causes large variations of idle connections counts on servers and
CPU spikes at the moment the cleanup task passes. These were quite more
visible with SSL as it costs CPU to close and re-establish these
connections, and it also takes time, reducing the reuse ratio, hence
increasing the amount of connections during reconnection.
In this patch we simply skip the killing loop when this condition is met.
The function timeofday_as_iso_us adds now the trailing local timezone offset.
Doing this the function could be use directly to generate rfc5424 logs.
It affects content of a ring if the ring's format is set to 'iso' and 'timed'.
Note: the default ring 'buf0' is of type 'timed'.
It is preferable NOT to backport this to stable releases unless bugs are
reported, because while the previous format is not correct and the new
one is correct, there is a small risk to cause inconsistencies in log
format to some users who would not expect such a change in a stable
cycle.
BUG/MAJOR: sched: make it work also when not building with DEBUG_STRICT
Sadly, the fix from commit 54d31170a ("BUG/MAJOR: sched: make sure
task_kill() always queues the task") broke the builds without DEBUG_STRICT
as, in order to be careful, it plcaed a BUG_ON() around the previously
failing condition to check for any new possible failure, but this BUG_ON
strips the condition when DEBUG_STRICT is not set. We don't want BUG_ON
to evaluate any condition either as some debugging code calls possibly
expensive ones (e.g. in htx_get_stline). Let's just drop the useless
BUG_ON().
BUILD: haproxy: fix build error when RLIMIT_AS is not set
As reported in issue #724, openbsd fails to build in haproxy.c
due to a faulty comma in the middle of a warning message. This code
is only compiled when RLIMIT_AS is not defined, which seems to be
rare these days.
This may be backported to older versions as the problem was likely
introduced when strict limits were added.
BUG/MEDIUM: cli/proxy: don't try to dump idle connection state if there's none
Commit 69f591e3b ("MINOR: cli/proxy: add a new "show servers conn" command")
added the ability to dump the idle connections state for a server, but we
must not do this if idle connections were not allocated, which happens if
the server is configured with pool-max-conn 0.
BUG/MEDIUM: muxes: Make sure nobody stole the connection before using it.
In the various timeout functions, make sure nobody stole the connection from
us before attempting to doing anything with it, there's a very small race
condition between the time we access the task context, and the time we
actually check it again with the lock, where it could have been free'd.
BUG/MAJOR: sched: make sure task_kill() always queues the task
task_kill() may fail to queue a task if this task has never ever run,
because its equivalent (tasklet->list) member has never been "emptied"
since it didn't pass through the LIST_DEL_INIT() that's performed by
run_tasks_from_lists(). This results in these tasks to never be freed.
It happens during the mux takeover since the target task usually is
the timeout task which, by definition, has never run yet.
This fixes commit eb8c2c69f ("MEDIUM: sched: implement task_kill() to
kill a task") which was introduced after 2.2-dev11 and doesn't need to
be backported.
projects / thirdparty / haproxy.git / log
