BUG/MEDIUM: task: do not destroy twice the tasks already killed in mworker_cleantasks()

Since 9228fabe34 ("BUG/MEDIUM: task: Do not destroy a task that is not ours
in task_destroy"), task_destroy() calls task_kill() for a task that is not
owned by the current thread. task_kill() is not idempotent: it starts with
BUG_ON(state & TASK_KILLED), so a task which was already flagged as killed
while still being linked in a run or wait queue cannot be destroyed again.

On the 3.0 and 2.8 branches this is reachable by simply starting haproxy in
master-worker mode with a localpeer, a peers section and a program section:
the forked program child calls mworker_cleanlisteners(), which destroys the
peers sync task, then mworker_cleantasks() walks the run queue and destroys
it a second time, aborting the child with SIGILL before it reaches execvp().
The program section no longer exists on this branch, but the underlying
invariant is still violated: a task flagged TASK_KILLED by an earlier
task_kill() is deliberately left in its queue (that is how the scheduler
knows it must free it on dequeue), so mworker_cleantasks() can always stumble
upon one. Make the walk robust instead of relying on the callers never
handing it a doomed task.

Fix this by ignoring the tasks that are already flagged TASK_KILLED when
walking the queues from mworker_cleantasks(). Those tasks don't need to be
destroyed again as they're already doomed: the scheduler frees them as
soon as it dequeues them (see run_tasks_from_lists()). The check is applied
to every queue walked there (the shared run queue, and the per-thread run
and timer queues).

This must be backported to all stable branches.

diff --git a/src/task.c b/src/task.c
index 0c7f5b8fd4b2822c7e870f4581070a95e94e2a87..9947f9678643af914207e28014ad1d359a182ea6 100644 (file)
--- a/src/task.c
+++ b/src/task.c
@@ -953,7 +953,8 @@ void mworker_cleantasks()
        while (tmp_rq) {
                t = eb32_entry(tmp_rq, struct task, rq);
                tmp_rq = eb32_next(tmp_rq);
-               task_destroy(t);
+               if (!(t->state & TASK_KILLED))
+                       task_destroy(t);
        }
 #endif
        /* clean the per thread run queue */
@@ -962,14 +963,16 @@ void mworker_cleantasks()
                while (tmp_rq) {
                        t = eb32_entry(tmp_rq, struct task, rq);
                        tmp_rq = eb32_next(tmp_rq);
-                       task_destroy(t);
+                       if (!(t->state & TASK_KILLED))
+                               task_destroy(t);
                }
                /* cleanup the per thread timers queue */
                tmp_wq = eb32_first(&ha_thread_ctx[i].timers);
                while (tmp_wq) {
                        t = eb32_entry(tmp_wq, struct task, wq);
                        tmp_wq = eb32_next(tmp_wq);
-                       task_destroy(t);
+                       if (!(t->state & TASK_KILLED))
+                               task_destroy(t);
                }
        }
 }