[PATCH] nptl: futex_lock_pi deadlock detection provides valuable information but it is turned into a rather cryptic assertion failure
Adhemerval Zanella Netto
adhemerval.zanella@linaro.org
Mon Apr 6 20:14:58 GMT 2026
On 02/04/26 14:09, Moritz KLAMMLER (FERCHAU) wrote:
> Hi Adhemerval,
>
> thanks for your assessment and please excuse the very long wait.
> Getting the copyright clearance from my employer to let me submit this
> patch took... a bit longer than expected. But we eventually did it.
Thanks for working on this, I looking forward for a v2.
>
> Attached is a conservative patch that will change pthread_mutex_lock
> only for recursive and error-checking PI mutexes to propagate the
> EDEADLK from the kernel to the user (these mutex types would previously
> have triggered the assertion). In my opinion, it would probably be more
> useful and easier to explain to propagate EDEADLK for all PI mutexes.
> But doing so gave me these two new test failures:
>
> nptl/tst-mutexpi6
> nptl/tst-thread-affinity-sched
>
> Apparently, those are depending on the deadlock actually happening for
> normal mutexes. Please help me understand whether this is behavior that
> we'd like/have to preserve. I'll be happy to adjust the patch
> accordingly.
The glibc defines PTHREAD_MUTEX_DEFAULT as PTHREAD_MUTEX_NORMAL:
sysdeps/nptl/pthread.h:
53 #if defined __USE_UNIX98 || defined __USE_XOPEN2K8
54 ,
55 PTHREAD_MUTEX_NORMAL = PTHREAD_MUTEX_TIMED_NP,
56 PTHREAD_MUTEX_RECURSIVE = PTHREAD_MUTEX_RECURSIVE_NP,
57 PTHREAD_MUTEX_ERRORCHECK = PTHREAD_MUTEX_ERRORCHECK_NP,
58 PTHREAD_MUTEX_DEFAULT = PTHREAD_MUTEX_NORMAL
59 #endif
60 #ifdef __USE_GNU
And PTHREAD_MUTEX_NORMAL is specified to deadlock in such cases [1].
[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/pthread_mutex_lock.html
>
> The added test case currently only checks for EDEADLK to be returned,
> but doesn't cover the cases where we still expect the deadlock to
> happen. I could add these as well, but it would introduce an
> (unreasonably?) long delay, waiting for the alarm clock to go off
> eventually. Would you rather take this delay over the lack of test
> coverage? Please also let me know whether having one test executable
> that loops over the various types (current patch) or one executable per
> type would be preferred.
I would prefer the later (one less binary to build and run). For the
deadlock to happen, it would be better to spawn a new process with
support_capture_subprocess, trigger the deadlock, and wait it with
short delayed_exit value.
>
> I've tried my best to be consistent with the code formatting, using the
> .clang-format file as far as this was applicable, but the space & tabs
> mixture gave me some trouble.+
The testcase should use libsupport, as below.
>
> Many thanks and best regards,
> Moritz
>
>
> From 618d71460ca8f66cd521df7bccc2fd16f8899335 Mon Sep 17 00:00:00 2001
> From: Moritz Klammler <moritz.klammler.ext@siemens.com>
> Date: Thu, 2 Apr 2026 18:10:01 +0200
> Subject: [PATCH 1/1] nptl: Propagate EDEADLK from FUTEX_LOCK_PI for
> errror-checking and recursive mutexes
>
> This patch changes the behavior of pthread_mutex_lock for error-checking and
> recursive PI mutexes in case of non-trivial deadlock. The user-space code
> doesn't detect the case where two or more threads would mutually deadlock each
> other, but the Linux kernel can. NPTL's previous behavior, if the syscall
> returns EDEADLK, was to run into an assertion. With this patch, the error code
> will be propagated to the caller who might then, at its own discretion and with
> knowledge about the application-level logic, use it to attempt resolving the
> situation gracefully or terminate the process after all.
>
> The behavior for other (normal) mutex types is not changed, they will continue
> to actually deadlock the calling thread.
>
> Since POSIX doesn't seem to mandate any particular behavior for this situation,
> and no existing code should have a dependency of running into an assertion,
> changing this behavior to what is presumably the most useful one seems to be
> justified.
>
> The previous (design) discussion can be seen here:
> https://sourceware.org/pipermail/libc-alpha/2025-December/173431.html
>
> Signed-off-by: Moritz Klammler <moritz.klammler.ext@siemens.com>
> ---
> nptl/Makefile | 1 +
> nptl/pthread_mutex_lock.c | 15 +++-
> nptl/tst-deadlk-pi.c | 2 +
> nptl/tst-deadlk.c | 183 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 198 insertions(+), 3 deletions(-)
> create mode 100644 nptl/tst-deadlk-pi.c
> create mode 100644 nptl/tst-deadlk.c
>
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 85f95dd0cf..41106677f7 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -283,6 +283,7 @@ tests = \
> tst-cleanup5 \
> tst-cond26 \
> tst-context1 \
> + tst-deadlk-pi \
> tst-default-attr \
> tst-dlsym1 \
> tst-exec4 \
> diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
> index a697f2b6ca..faf53d44fe 100644
> --- a/nptl/pthread_mutex_lock.c
> +++ b/nptl/pthread_mutex_lock.c
> @@ -418,9 +418,18 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
> NULL, private);
> if (e == ESRCH || e == EDEADLK)
> {
> - assert (e != EDEADLK
> - || (kind != PTHREAD_MUTEX_ERRORCHECK_NP
> - && kind != PTHREAD_MUTEX_RECURSIVE_NP));
> + if (e == EDEADLK
> + && (kind == PTHREAD_MUTEX_ERRORCHECK_NP
> + || kind == PTHREAD_MUTEX_RECURSIVE_NP))
> + {
> + /* FUTEX_LOCK_PI may return EDEADLK due to cross‑thread
> + * deadlock detection, beyond the same‑thread recursive
> + * check above. Pass this error through for these two
> + * mutex types; otherwise, intentionally deadlock for
> + * normal mutexes. */
The usual comment format is to no use '*' as the start of new line:
/* FUTEX_LOCK_PI may return EDEADLK due to cross‑thread
deadlock detection, beyond the same‑thread recursive
[...]
> + return e;
> + }
> +
> /* ESRCH can happen only for non-robust PI mutexes where
> the owner of the lock died. */
> assert (e != ESRCH || !robust);
> diff --git a/nptl/tst-deadlk-pi.c b/nptl/tst-deadlk-pi.c
> new file mode 100644
> index 0000000000..3196a546d0
> --- /dev/null
> +++ b/nptl/tst-deadlk-pi.c
> @@ -0,0 +1,2 @@
> +#define TST_DEADLK_MUTEX_PI 1
> +#include "tst-deadlk.c"
> diff --git a/nptl/tst-deadlk.c b/nptl/tst-deadlk.c
> new file mode 100644
> index 0000000000..3a9ea8ee44
> --- /dev/null
> +++ b/nptl/tst-deadlk.c
> @@ -0,0 +1,183 @@
> +/* This test checks behavior not required by POSIX. */
> +/* https://sourceware.org/pipermail/libc-alpha/2025-December/173431.html */
This need a Copyright header, along with a one-line description (first line)
of what tests intendes.
> +
> +#include <errno.h>
> +#include <pthread.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <support/test-driver.h>
This can be simplified to:
#include <array_length.h>
#include <errno.h>
#include <stdint.h>
#include <stdio.h>
#include <support/xthread.h>
#include <support/check.h>
#include <support/test-driver.h>
> +
> +#ifndef TST_DEADLK_TIMEOUT
> +# define TST_DEADLK_TIMEOUT 10
> +#endif
No need to handle timeout, support/test-driver.c already does that.
> +
> +#ifndef TST_DEADLK_MUTEX_PI
> +# define TST_DEADLK_MUTEX_PI 0
> +#endif
Just use the TST_DEADLK_MUTEX_PI=1 and move this test to tst-deadlk-pi.c.
> +
> +#define ARRAY_SIZE(Array) (sizeof (Array) / sizeof ((Array)[0]))
Use array_length instead.
> +
> +#define CALL_PTHREAD_OR_FAIL(Func, ...) \
> + CALL_PTHREAD_OR_EXIT (EXIT_FAILURE, Func, __VA_ARGS__)
> +
> +#define CALL_PTHREAD_OR_SKIP(Func, ...) \
> + CALL_PTHREAD_OR_EXIT (EXIT_UNSUPPORTED, Func, __VA_ARGS__)
> +
> +#define CALL_PTHREAD_OR_EXIT(Status, Func, ...) \
> + do \
> + { \
> + const int ret = Func (__VA_ARGS__); \
> + if (ret > 0) \
> + { \
> + printf ("%s:%d: %s returned positive status %d: %s", __FILE__, \
> + __LINE__, #Func, ret, strerror (ret)); \
> + exit (Status); \
> + } \
> + } \
> + while (false)
There is no need of any of these macros.
> +
> +struct howto_test
> +{
> + int type;
> + bool robust;
> + bool prio_inherit;
> +};
> +
> +struct task_context
> +{
> + pthread_mutex_t *first, *second;
> + pthread_barrier_t *barrier;
> +};
> +
> +static const struct howto_test howto[] = {
> + { .type = PTHREAD_MUTEX_ERRORCHECK,
> + .prio_inherit = TST_DEADLK_MUTEX_PI,
> + .robust = false },
> + { .type = PTHREAD_MUTEX_ERRORCHECK,
> + .prio_inherit = TST_DEADLK_MUTEX_PI,
> + .robust = true },
> + { .type = PTHREAD_MUTEX_RECURSIVE,
> + .prio_inherit = TST_DEADLK_MUTEX_PI,
> + .robust = false },
> + { .type = PTHREAD_MUTEX_RECURSIVE,
> + .prio_inherit = TST_DEADLK_MUTEX_PI,
> + .robust = true },
> +};
> +
> +static void *
> +thread_function (void *const arg)
> +{
> + const struct task_context *ctx = arg;
> + intptr_t ret = 0;
> + CALL_PTHREAD_OR_FAIL (pthread_mutex_lock, ctx->first);
> + CALL_PTHREAD_OR_FAIL (pthread_barrier_wait, ctx->barrier);
> + ret = pthread_mutex_lock (ctx->second);
> + CALL_PTHREAD_OR_FAIL (pthread_mutex_unlock, ctx->first);
> + if (ret == 0)
> + CALL_PTHREAD_OR_FAIL (pthread_mutex_unlock, ctx->second);
> + return (void *) ret;
> +}
This can be simplified to:
static void *
thread_function (void *const arg)
{
const struct task_context *ctx = arg;
intptr_t ret = 0;
xpthread_mutex_lock (ctx->first);
xpthread_barrier_wait (ctx->barrier);
ret = pthread_mutex_lock (ctx->second);
xpthread_mutex_unlock (ctx->first);
if (ret == 0)
xpthread_mutex_unlock (ctx->second);
return (void *) ret;
}
> +
> +static void
> +initialize_mutex_or_skip_test (pthread_mutex_t *const mutex,
> + const struct howto_test *const how)
> +{
> + pthread_mutexattr_t attr;
> + CALL_PTHREAD_OR_SKIP (pthread_mutexattr_init, &attr);
> + CALL_PTHREAD_OR_SKIP (pthread_mutexattr_settype, &attr, how->type);
> + if (how->robust)
> + {
> + CALL_PTHREAD_OR_SKIP (pthread_mutexattr_setrobust, &attr,
> + PTHREAD_MUTEX_ROBUST);
> + }
> + if (how->prio_inherit)
> + {
> + CALL_PTHREAD_OR_SKIP (pthread_mutexattr_setprotocol, &attr,
> + PTHREAD_PRIO_INHERIT);
> + }
> + CALL_PTHREAD_OR_SKIP (pthread_mutex_init, mutex, &attr);
> + CALL_PTHREAD_OR_SKIP (pthread_mutexattr_destroy, &attr);
> +}
And this to:
static void
initialize_mutex_or_skip_test (pthread_mutex_t *const mutex,
const struct howto_test *const how)
{
pthread_mutexattr_t attr;
xpthread_mutexattr_init (&attr);
xpthread_mutexattr_settype (&attr, how->type);
if (how->robust)
xpthread_mutexattr_setrobust (&attr, PTHREAD_MUTEX_ROBUST);
if (how->prio_inherit)
xpthread_mutexattr_setprotocol (&attr, PTHREAD_PRIO_INHERIT);
xpthread_mutex_init (mutex, &attr);
xpthread_mutexattr_destroy (&attr);
}
> +
> +static void
> +beforehand (const struct howto_test *const how)
> +{
> + printf (
> + "Testing with this mutex: type = %d, robust = %d, prio_inherit = %d\n",
> + how->type, how->robust, how->prio_inherit);
> +}
> +
> +static int
> +analyze_results (const struct howto_test *const how, const int ret1,
> + const int ret2)
> +{
> + if ((ret1 != EDEADLK) && (ret2 != EDEADLK))
> + {
> + printf ("At least one thread should have gotten %d but "
> + "threads got %d and %d respectively.\n",
> + EDEADLK, ret1, ret2);
> + return EXIT_FAILURE;
> + }
> + else if (ret1 != 0 && ret1 != EDEADLK)
> + {
> + printf ("First thread should have gotten 0 or %d but got %d "
> + "instead.\n",
> + EDEADLK, ret1);
> + return EXIT_FAILURE;
> + }
> + else if (ret2 != 0 && ret2 != EDEADLK)
> + {
> + printf ("Second thread should have gotten 0 or %d but got %d "
> + "instead.\n",
> + EDEADLK, ret2);
> + return EXIT_FAILURE;
> + }
> + else
> + {
> + printf ("Threads got %d and %d respectively which is in line with the "
> + "expectation.\n",
> + ret1, ret2);
> + return EXIT_SUCCESS;
> + }
> +}
And this to:
static void
analyze_results (const struct howto_test *const how, const int ret1,
const int ret2)
{
if ((ret1 != EDEADLK) && (ret2 != EDEADLK))
FAIL_EXIT1 ("At least one thread should have gotten %d but "
"threads got %d and %d respectively.\n",
EDEADLK, ret1, ret2);
else if (ret1 != 0 && ret1 != EDEADLK)
FAIL_EXIT1 ("First thread should have gotten 0 or %d but got %d "
"instead.\n",
EDEADLK, ret1);
else if (ret2 != 0 && ret2 != EDEADLK)
FAIL_EXIT1 ("Second thread should have gotten 0 or %d but got %d "
"instead.\n",
EDEADLK, ret2);
else
printf ("Threads got %d and %d respectively which is in line with the "
"expectation.\n",
ret1, ret2);
}
> +
> +static int
> +do_test (void)
> +{
> + for (size_t i = 0; i < ARRAY_SIZE (howto); ++i)
> + {
> + pthread_t t1, t2;
> + pthread_mutex_t m1, m2;
> + void *ret1, *ret2;
> + pthread_barrier_t barrier;
> + struct task_context ctx1
> + = { .first = &m1, .second = &m2, .barrier = &barrier };
> + struct task_context ctx2
> + = { .first = &m2, .second = &m1, .barrier = &barrier };
> + beforehand (howto + i);
> + alarm (TST_DEADLK_TIMEOUT);
> + CALL_PTHREAD_OR_FAIL (pthread_barrier_init, &barrier, NULL, 2);
> + initialize_mutex_or_skip_test (&m1, howto + i);
> + initialize_mutex_or_skip_test (&m2, howto + i);
> + CALL_PTHREAD_OR_FAIL (pthread_create, &t1, NULL, thread_function, &ctx1);
> + CALL_PTHREAD_OR_FAIL (pthread_create, &t2, NULL, thread_function, &ctx2);
> + CALL_PTHREAD_OR_FAIL (pthread_join, t1, &ret1);
> + CALL_PTHREAD_OR_FAIL (pthread_join, t2, &ret2);
> + CALL_PTHREAD_OR_FAIL (pthread_mutex_destroy, &m1);
> + CALL_PTHREAD_OR_FAIL (pthread_mutex_destroy, &m2);
> + CALL_PTHREAD_OR_FAIL (pthread_barrier_destroy, &barrier);
> + alarm (0);
> + const int verdict
> + = analyze_results (howto + i, (intptr_t) ret1, (intptr_t) ret2);
> + if (verdict != 0)
> + return verdict;
> + }
> + return 0;
> +}
> +
And this to:
static int
do_test (void)
{
for (size_t i = 0; i < array_length (howto); ++i)
{
pthread_mutex_t m1, m2;
pthread_barrier_t barrier;
struct task_context ctx1
= { .first = &m1, .second = &m2, .barrier = &barrier };
struct task_context ctx2
= { .first = &m2, .second = &m1, .barrier = &barrier };
beforehand (howto + i);
xpthread_barrier_init (&barrier, NULL, 2);
initialize_mutex_or_skip_test (&m1, howto + i);
initialize_mutex_or_skip_test (&m2, howto + i);
pthread_t t1 = xpthread_create (NULL, thread_function, &ctx1);
pthread_t t2 = xpthread_create (NULL, thread_function, &ctx2);
void *ret1 = xpthread_join (t1);
void *ret2 = xpthread_join (t2);
xpthread_mutex_destroy (&m1);
xpthread_mutex_destroy (&m2);
xpthread_barrier_destroy (&barrier);
analyze_results (howto + i, (intptr_t) ret1, (intptr_t) ret2);
}
return 0;
}
> +#include <support/test-driver.c>
More information about the Libc-alpha
mailing list