MINOR: tasks: catch TICK_ETERNITY with BUG_ON() in __task_queue()
__task_queue() must absolutely not be called with TICK_ETERNITY or it
will place a never-expiring node upfront in the timers queue, preventing
any timer from expiring until the process is restarted. Code was found
to cause this using "task_schedule(task, now_ms)" which does this one
millisecond every 49.7 days, so let's add a condition against this. It
must never trigger since any process susceptible to trigger it would
already accumulate tasks until it dies.
An extra test was added in wake_expired_tasks() to detect tasks whose
timeout would have been changed after being queued.
An improvement over this could be in the future to use a non-scalar
type (union/struct) for expiration dates so as to avoid the risk of
using them directly like this. But now_ms is already such a valid
time and this specific construct would still not be caught.
This could even be backported to stable versions to help detect other
occurrences if any.
(cherry picked from commit 7a9699916a92a98ca5daaff77e2a25f9bec8817d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/task.c b/src/task.c
index c3145b0..8b08675 100644
--- a/src/task.c
+++ b/src/task.c
@@ -213,7 +213,7 @@
*
* Inserts a task into wait queue <wq> at the position given by its expiration
* date. It does not matter if the task was already in the wait queue or not,
- * as it will be unlinked. The task must not have an infinite expiration timer.
+ * as it will be unlinked. The task MUST NOT have an infinite expiration timer.
* Last, tasks must not be queued further than the end of the tree, which is
* between <now_ms> and <now_ms> + 2^31 ms (now+24days in 32bit).
*
@@ -230,6 +230,10 @@
(wq == &sched->timers && (task->state & TASK_SHARED_WQ)) ||
(wq != &timers && wq != &sched->timers));
#endif
+ /* if this happens the process is doomed anyway, so better catch it now
+ * so that we have the caller in the stack.
+ */
+ BUG_ON(task->expire == TICK_ETERNITY);
if (likely(task_in_wq(task)))
__task_unlink_wq(task);
@@ -303,7 +307,8 @@
__task_queue(task, &tt->timers);
}
else {
- /* task not expired and correctly placed */
+ /* task not expired and correctly placed. It may not be eternal. */
+ BUG_ON(task->expire == TICK_ETERNITY);
break;
}
}
@@ -373,7 +378,8 @@
goto lookup_next;
}
else {
- /* task not expired and correctly placed */
+ /* task not expired and correctly placed. It may not be eternal. */
+ BUG_ON(task->expire == TICK_ETERNITY);
break;
}
}