From 45e83e8e81ca76d91a4e9b3e5d065e9d78ba4d57 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 2 May 2025 10:34:16 +0200 Subject: [PATCH] BUG/MAJOR: tasks: fix task accounting when killed After recent commit b81c9390f ("MEDIUM: tasks: Mutualize the TASK_KILLED code between tasks and tasklets"), the task accounting was no longer correct for killed tasks due to the decrement of tasks in list that was no longer done, resulting in infinite loops in process_runnable_tasks(). This just illustrates that this code remains complex and should be further cleaned up. No backport is needed, as this was in 3.2. --- src/task.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/task.c b/src/task.c index 664742704..843e46790 100644 --- a/src/task.c +++ b/src/task.c @@ -613,6 +613,19 @@ unsigned int run_tasks_from_lists(unsigned int budgets[]) } } while (!_HA_ATOMIC_CAS(&t->state, &state, (state & TASK_PERSISTENT) | TASK_RUNNING)); + __ha_barrier_atomic_store(); + + /* keep the task counter up to date */ + if (!(t->state & TASK_F_TASKLET)) + _HA_ATOMIC_DEC(&ha_thread_ctx[tid].tasks_in_list); + + /* From this point, we know that the task or tasklet was properly + * dequeued, flagged and accounted for. Let's now check if it was + * killed. If TASK_KILLED arrived before we've read the state, we + * directly free the task/tasklet. Otherwise for tasks it will be + * seen after processing and it's freed on the exit path. + */ + if (unlikely((state & TASK_KILLED) || process == NULL)) { /* Task or tasklet has been killed, let's remove it */ if (t->state & TASK_F_TASKLET) @@ -627,6 +640,8 @@ unsigned int run_tasks_from_lists(unsigned int budgets[]) */ goto next; } + + /* OK now the task or tasklet is well alive and is going to be run */ if (t->state & TASK_F_TASKLET) { /* this is a tasklet */ @@ -635,21 +650,14 @@ unsigned int run_tasks_from_lists(unsigned int budgets[]) _HA_ATOMIC_AND(&t->state, ~TASK_RUNNING); } else { /* This is a regular task */ - __ha_barrier_atomic_store(); - _HA_ATOMIC_DEC(&ha_thread_ctx[tid].tasks_in_list); - - /* Note for below: if TASK_KILLED arrived before we've read the state, we - * directly free the task. Otherwise it will be seen after processing and - * it's freed on the exit path. - */ if (process == process_stream) t = process_stream(t, ctx, state); else t = process(t, ctx, state); - /* If there is a pending state we have to wake up the task - * immediately, else we defer it into wait queue + /* If there is a pending state, we have to wake up the task + * immediately, else we defer it into wait queue. */ if (t != NULL) { state = _HA_ATOMIC_LOAD(&t->state);