RepoMirrors
/
haproxy
mirror of http://git.haproxy.org/git/haproxy.git/
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

MEDIUM: servers: make the server deletion code run under full thread isolation 

In 2.4, runtime server deletion was brought by commit e558043e1 ("MINOR:
server: implement delete server cli command"). A comment remained in the
code about a theoretical race between the thread_isolate() call and another
thread being in the process of allocating memory before accessing the
server via a reference that was grabbed before the memory allocation,
since the thread_harmless_now()/thread_harmless_end() pair around mmap()
may have the effect of allowing cli_parse_delete_server() to proceed.

Now that the full thread isolation is available, let's update the code
to rely on this. Now it is guaranteed that competing threads will either
be in the poller or queued in front of thread_isolate_full().

This may be backported to 2.4 if any report of breakage suggests the bug
really exists, in which case the two following patches will also be
needed:
  MINOR: threads: make thread_release() not wait for other ones to complete
  MEDIUM: threads: add a stronger thread_isolate_full() call
This commit is contained in:
Willy Tarreau 2021-08-04 14:42:37 +02:00
parent 88d1c5d3fb
commit ba3ab7907a
1 changed files with 5 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/server.c
View File

@ -4631,19 +4631,12 @@ static int cli_parse_delete_server(char **args, char *payload, struct appctx *ap
return cli_err(appctx, "Require 'backend/server'.");
/* The proxy servers list is currently not protected by a lock so this
* requires thread isolation.
* requires thread isolation. In addition, any place referencing the
* server about to be deleted would be unsafe after our operation, so
* we must be certain to be alone so that no other thread has even
* started to grab a temporary reference to this server.
*/
/* WARNING there is maybe a potential violation of the thread isolation
* mechanism by the pool allocator. The allocator marks the thread as
* harmless before the allocation, but a processing outside of it could
* relies on a particular server triggered at the same time by a
* 'delete server'. Currently, it is unknown if such case is present in
* the current code. If it happens to be, the thread isolation
* mechanism should be improved, maybe with a differentiation between
* read and read+write safe sections.
*/
thread_isolate();
thread_isolate_full();
get_backend_server(be_name, sv_name, &be, &srv);
if (!be) {