From: Aurelien DARRAGON <adarragon@haproxy.com>
Date: Mon, 15 Jul 2024 13:44:49 +0000 (+0200)
Subject: MINOR: server: better mt_list usage for node migration (prev_deleted handling)
X-Git-Tag: v3.1-dev4~56
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=05f33e95ba5e05a2600d6e95e6ad8be6f950912a;p=thirdparty%2Fhaproxy.git

MINOR: server: better mt_list usage for node migration (prev_deleted handling)

Now that mt_list v2 api was merged into haproxy's codebase in 4e65fc6 ("
MAJOR: import: update mt_list to support exponential back-off (try #2)"),
let's fix a hack in cli_parse_delete_server() which abused from mt_list
api to migrate an element from one list to another: there used to be a
tiny race there between the pop and the append operations, race that was
compensated by the fact that it was performed under full thread isolation.

However that was a bad example of the mt_list API which could have
resulted in actual bug if the code was duplicated elsewhere without thread
isolation. To fix this, we now make use of the
MT_LIST_FOR_EACH_ENTRY_LOCKED() macro which allows us to simply migrate
the current element to another list since the element is appended into
another one while still in busy state and then unlinked from the original
list.
---

diff --git a/src/server.c b/src/server.c
index 3a667095b4..e41c2eb885 100644
--- a/src/server.c
+++ b/src/server.c
@@ -6108,18 +6108,19 @@ static int cli_parse_delete_server(char **args, char *payload, struct appctx *ap
 	_srv_detach(srv);
 
 	/* Some deleted servers could still point to us using their 'next',
-	 * update them as needed
-	 * Please note the small race between the POP and APPEND, although in
-	 * this situation this is not an issue as we are under full thread
-	 * isolation
+	 * migrate them as needed
 	 */
-	while ((prev_del = MT_LIST_POP(&srv->prev_deleted, struct server *, prev_deleted))) {
+	MT_LIST_FOR_EACH_ENTRY_LOCKED(prev_del, &srv->prev_deleted, prev_deleted, back) {
 		/* update its 'next' ptr */
 		prev_del->next = srv->next;
 		if (srv->next) {
 			/* now it is our 'next' responsibility */
 			MT_LIST_APPEND(&srv->next->prev_deleted, &prev_del->prev_deleted);
 		}
+		else
+			mt_list_unlock_self(&prev_del->prev_deleted);
+		/* unlink from our list */
+		prev_del = NULL;
 	}
 
 	/* we ourselves need to inform our 'next' that we will still point it */