mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-21 05:41:26 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MEDIUM: server: ensure thread-safety of server runtime creation

Browse Source 
cli_parse_add_server can be executed in parallel by several CLI
instances and so must be thread-safe. The critical points of the
function are :
- server duplicate detection
- insertion of the server in the proxy list

The mode of operation has been reversed. The server is first
instantiated and parsed. The duplicate check has been moved at the end
just before the insertion in the proxy list, under the thread isolation.
Thus, the thread safety is guaranteed and server allocation is kept
outside of locks/thread isolation.
This commit is contained in:
Amaury Denoyelle 2021-04-20 17:09:08 +02:00
parent d688e01032
commit cece918625
1 changed files with 24 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/server.c
View File

@ -4347,7 +4347,7 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
if (!*sv_name)
return cli_err(appctx, "Require 'backend/server'.");
get_backend_server(be_name, sv_name, &be, &srv);
be = proxy_be_by_name(be_name);
if (!be)
return cli_err(appctx, "No such backend.");
@ -4356,9 +4356,6 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
return 1;
}
if (srv)
return cli_err(appctx, "Already exists a server with the same name in backend.");
args[1] = sv_name;
errcode = _srv_parse_init(&srv, args, &argc, be, parse_flags, &errmsg);
if (errcode) {
@ -4423,17 +4420,33 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
goto out;
}
/* attach the server to the end of proxy linked list
/* Attach the server to the end of the proxy linked list. The proxy
* servers list is currently not protected by a lock, so this requires
* thread_isolate/release.
*
* The proxy servers list is currently not protected by a lock, so this
* requires thread_isolate/release.
* If a server with the same name is found, reject the new one. This
* operation requires thread-safety and thus cannot be executed at the
* beginning without having server allocation under locks/isolation.
*/
thread_isolate();
/* TODO use a double-linked list for px->srv */
if (be->srv) {
struct server *next;
for (next = be->srv; next->next; next = next->next)
;
struct server *next = be->srv;
while (1) {
/* check for duplicate server */
if (!strcmp(srv->id, next->id)) {
thread_release();
cli_err(appctx, "Already exists a server with the same name in backend.");
goto out;
}
if (!next->next)
break;
next = next->next;
}
next->next = srv;
}
@ -4441,6 +4454,7 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
srv->next = be->srv;
be->srv = srv;
}
thread_release();
cli_msg(appctx, LOG_INFO, "New server registered.");