]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
CLEANUP: task: move the barrier after clearing th_ctx->current
authorWilly Tarreau <w@1wt.eu>
Fri, 21 Feb 2025 10:22:57 +0000 (11:22 +0100)
committerWilly Tarreau <w@1wt.eu>
Fri, 21 Feb 2025 17:31:46 +0000 (18:31 +0100)
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.

src/task.c

index 9b6beb8ad71dfc06d19bbaa06d102eeafcd7aefb..02b4ef5bb72415f6d5b9d3b97c6fff936579f0be 100644 (file)
@@ -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 {