From: Willy Tarreau <w@1wt.eu>
Date: Fri, 21 Feb 2025 10:22:57 +0000 (+0100)
Subject: CLEANUP: task: move the barrier after clearing th_ctx->current
X-Git-Tag: v3.2-dev7~71
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e7510d62306ec4152db3c171744e2e1ef6b8de63;p=thirdparty%2Fhaproxy.git

CLEANUP: task: move the barrier after clearing th_ctx->current

There's a barrier after releasing the current task in the scheduler.
However it's improperly placed, it's done after pool_free() while in
fact it must be done immediately after resetting the current pointer.
Indeed, the purpose is to make sure that nobody sees the task as valid
when it's in the process of being released. This is something that
could theoretically happen if interrupted by a signal in the inlined
code of pool_free() if the compiler decided to postpone the write to
->current. In practice since nothing fancy is done in the inlined part
of the function, there's currently no risk of reordering. But it could
happen if the underlying __pool_free() were to be inlined for example,
and in this case we could possibly observe th_ctx->current pointing
to something currently being destroyed.

With the barrier between the two, there's no risk anymore.
---

diff --git a/src/task.c b/src/task.c
index 9b6beb8ad..02b4ef5bb 100644
--- a/src/task.c
+++ b/src/task.c
@@ -605,8 +605,11 @@ unsigned int run_tasks_from_lists(unsigned int budgets[])
 			else {
 				done++;
 				th_ctx->current = NULL;
-				pool_free(pool_head_tasklet, t);
+				/* signal barrier to prevent thread dump helpers
+				 * from dumping a task currently being freed.
+				 */
 				__ha_barrier_store();
+				pool_free(pool_head_tasklet, t);
 				continue;
 			}
 		} else {