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:
parent
88d1c5d3fb
commit
ba3ab7907a
17
src/server.c
17
src/server.c
|
@ -4631,19 +4631,12 @@ static int cli_parse_delete_server(char **args, char *payload, struct appctx *ap
|
||||||
return cli_err(appctx, "Require 'backend/server'.");
|
return cli_err(appctx, "Require 'backend/server'.");
|
||||||
|
|
||||||
/* The proxy servers list is currently not protected by a lock so this
|
/* 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.
|
||||||
*/
|
*/
|
||||||
|
thread_isolate_full();
|
||||||
/* 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();
|
|
||||||
|
|
||||||
get_backend_server(be_name, sv_name, &be, &srv);
|
get_backend_server(be_name, sv_name, &be, &srv);
|
||||||
if (!be) {
|
if (!be) {
|
||||||
|
|
Loading…
Reference in New Issue