From: Christopher Faulet <cfaulet@haproxy.com>
Date: Wed, 6 Jan 2021 06:41:56 +0000 (+0100)
Subject: BUG/MINOR: stats: Make stat_l variable used to dump a stat line thread local
X-Git-Tag: v2.4-dev5~9
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=de79cd28ec9e1facc5f2da02cd95dda82ac6fe58;p=thirdparty%2Fhaproxy.git

BUG/MINOR: stats: Make stat_l variable used to dump a stat line thread local

Since ee63d4bd6 ("MEDIUM: stats: integrate static proxies stats in new
stats"), all dumped stats for a given domain, the default ones and the
modules ones, are merged in a signle array to dump them in a generic way.
For this purpose, the stat_l global variable is allocated at startup to
store a line of stats before the dump, i.e. all stats of an entity
(frontend, backend, listener, server or dns nameserver). But this variable
is not thread safe.  If stats are retrieved concurrently by several clients
on different threads, the same variable is used. This leads to corrupted
stats output.

To fix the bug, the stat_l variable is now thread local.

This patch should probably solve issues #972 and #992. It must be backported
to 2.3.
---

diff --git a/src/stats.c b/src/stats.c
index e2d37779bf..36e5c077a6 100644
--- a/src/stats.c
+++ b/src/stats.c
@@ -261,7 +261,7 @@ static struct name_desc *stat_f[STATS_DOMAIN_COUNT];
 static size_t stat_count[STATS_DOMAIN_COUNT];
 
 /* one line for stats */
-static struct field *stat_l[STATS_DOMAIN_COUNT];
+static THREAD_LOCAL struct field *stat_l[STATS_DOMAIN_COUNT];
 
 /* list of all registered stats module */
 static struct list stats_module_list[STATS_DOMAIN_COUNT] = {
@@ -4500,12 +4500,7 @@ static int allocate_stats_px_postcheck(void)
 		}
 	}
 
-	stat_l[STATS_DOMAIN_PROXY] = malloc(stat_count[STATS_DOMAIN_PROXY] * sizeof(struct field));
-	if (!stat_l[STATS_DOMAIN_PROXY]) {
-		ha_alert("stats: cannot allocate a line for proxy statistics\n");
-		err_code |= ERR_ALERT | ERR_FATAL;
-		return err_code;
-	}
+	/* wait per-thread alloc to perform corresponding stat_l allocation */
 
 	return err_code;
 }
@@ -4538,18 +4533,29 @@ static int allocate_stats_dns_postcheck(void)
 		return err_code;
 	}
 
-	stat_l[STATS_DOMAIN_DNS] = malloc(stat_count[STATS_DOMAIN_DNS] * sizeof(struct field));
-	if (!stat_l[STATS_DOMAIN_DNS]) {
-		ha_alert("stats: cannot allocate a line for dns statistics\n");
-		err_code |= ERR_ALERT | ERR_FATAL;
-		return err_code;
-	}
+	/* wait per-thread alloc to perform corresponding stat_l allocation */
 
 	return err_code;
 }
 
 REGISTER_CONFIG_POSTPARSER("allocate-stats-dns", allocate_stats_dns_postcheck);
 
+static int allocate_stat_lines_per_thread(void)
+{
+	int domains[] = { STATS_DOMAIN_PROXY, STATS_DOMAIN_DNS }, i;
+
+	for (i = 0; i < STATS_DOMAIN_COUNT; ++i) {
+		const int domain = domains[i];
+
+		stat_l[domain] = malloc(stat_count[domain] * sizeof(struct field));
+		if (!stat_l[domain])
+			return 0;
+	}
+	return 1;
+}
+
+REGISTER_PER_THREAD_ALLOC(allocate_stat_lines_per_thread);
+
 static int allocate_trash_counters(void)
 {
 	struct stats_module *mod;
@@ -4578,15 +4584,27 @@ static int allocate_trash_counters(void)
 
 REGISTER_PER_THREAD_ALLOC(allocate_trash_counters);
 
-static void deinit_stats(void)
+static void deinit_stat_lines_per_thread(void)
 {
 	int domains[] = { STATS_DOMAIN_PROXY, STATS_DOMAIN_DNS }, i;
 
 	for (i = 0; i < STATS_DOMAIN_COUNT; ++i) {
 		const int domain = domains[i];
 
-		if (stat_l[domain])
-			free(stat_l[domain]);
+		free(stat_l[domain]);
+		stat_l[domain] = NULL;
+	}
+}
+
+
+REGISTER_PER_THREAD_FREE(deinit_stat_lines_per_thread);
+
+static void deinit_stats(void)
+{
+	int domains[] = { STATS_DOMAIN_PROXY, STATS_DOMAIN_DNS }, i;
+
+	for (i = 0; i < STATS_DOMAIN_COUNT; ++i) {
+		const int domain = domains[i];
 
 		if (stat_f[domain])
 			free(stat_f[domain]);