BUG/MEDIUM: server: ensure thread-safety of server runtime creation
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.
diff --git a/src/server.c b/src/server.c
index 78620e0..ac53221 100644
--- a/src/server.c
+++ b/src/server.c
@@ -4347,7 +4347,7 @@
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 @@
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 @@
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 @@
srv->next = be->srv;
be->srv = srv;
}
+
thread_release();
cli_msg(appctx, LOG_INFO, "New server registered.");