BUG/MEDIUM: shctx: really check the lock's value while waiting
authorWilly Tarreau <w@1wt.eu>
Fri, 1 May 2020 11:05:29 +0000 (13:05 +0200)
committerWilly Tarreau <w@1wt.eu>
Fri, 1 May 2020 11:33:13 +0000 (13:33 +0200)
commitdef4271bb22207983d0d17a8f02e99dcb6b60fa7
tree3435b4668583d2b10403de7c4d59998dcd681588
parentc7e86ea2f02c669961a2bd8f7bfc137deaef3cb6
BUG/MEDIUM: shctx: really check the lock's value while waiting

Jérôme reported an amazing crash in the spinlock version of
_shctx_wait4lock() with an extremely high <count> value of 32M! The
root cause is that the function cannot deal with contention on the lock
at all because it forgets to check if the lock's value has changed! As
such, every time it's called due to a contention, it waits twice as
long before trying again and lets the caller check for the contention
by itself.

The correct thing to do is to compare the value again at each loop.
This way it makes sure to mostly perform read accesses on the shared
cache line without writing too often, and to be ready fast enough to
try to grab the lock. And we must not increase the count on success
either!

Unfortunately I'd have expected to see a performance boost on the cache
with this but there was absolutely no change, so it's very likely that
these issues only happen once in a while and are sufficient to derail
the process when they strike, but not to have a permanent performance
impact.

The bug was introduced with the shctx entries in 1.5 so the fix must
be backported to all versions. Before 1.8 the function was called
_shared_context_wait4lock() and was in shctx.c.

(cherry picked from commit 3801bdc3fc1c2c6d596b4c4ee3449a76f2da8654)
Signed-off-by: Willy Tarreau <w@1wt.eu>
include/proto/shctx.h