BUG/MINOR: applet: accept a temporarily owned tid in appctx_init()

appctx_init() makes sure the appctx is initialized on the thread it is bound
to, and checks it with a BUG_ON() which tolerates an unbound task (tid ==
-1), since an appctx may be created without a target thread and left to
whichever thread handles it first. That tolerance was added by commit
4f7c26cbb ("BUG/MINOR: applet: Don't trigger BUG_ON if the tid is not on
appctx init") for 2.8, at a time when a task's thread ID was either a real
thread number or -1.

Since commit 8b6d8f5e4 ("MINOR: tasks: Add __task_get_new_tid_field()"), the
scheduler encodes the temporary ownership of a shared task differently: when
a thread takes ownership of a task whose tid is -1, it sets -2 - <thread>
instead, so that no other thread can grab it before it is queued or
run. That value is still "not bound" but designates a specific owner, and it
is what task_wakeup() and task_queue() now set through
__task_get_new_tid_field(). The task code already knows about it:
__tasklet_wakeup_here() carries the very same triple test, and
task_set_thread() has an explicit early return for "t->tid == -2 - tid" when
the requested thread is -1.

appctx_init() did not, so it would trip the BUG_ON() on a task that happens
to be in that transient state, instead of adopting the current thread as
task_set_thread() is about to do. Let's accept -2 - tid here too, so that
the three states handled by task_set_thread() are all accepted: unbound,
bound to the current thread, or temporarily owned by the current thread.

This is not reachable today: every appctx is created with an explicit target
thread (appctx_new_here() or appctx_new_on() with a valid thread number), so
its task tid is never -1 and never -2 - tid, and appctx_new_anywhere()
currently has no caller. It is nonetheless a latent bug: any future user of
appctx_new_anywhere(), which is the intended way to start an appctx on a
thread subset, would hit the BUG_ON() as soon as the task is woken up before
being initialized.

The "-2 - tid" encoding was introduced in 3.5-dev1, so no backport needed.

diff --git a/include/haproxy/applet.h b/include/haproxy/applet.h
index 27377c574da55ab4ea21719cb3be9bcd5b48de29..762a7056e77ed72fc5a8d7f4a78a88748b23fcfe 100644 (file)
--- a/include/haproxy/applet.h
+++ b/include/haproxy/applet.h
@@ -129,7 +129,7 @@ static inline int appctx_init(struct appctx *appctx)
         * the appctx will be fully initialized. The session and the stream will
         * eventually be created. The affinity must be set now !
         */
-       BUG_ON(appctx->t->tid != -1 && appctx->t->tid != tid);
+       BUG_ON(appctx->t->tid != -1 && appctx->t->tid != tid && appctx->t->tid != -2 - tid);
        task_set_thread(appctx->t, tid);
 
        if (appctx->applet->init)