nptl: futex_lock_pi deadlock detection provides valuable information but it is turned into a rather cryptic assertion failure

  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.

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 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'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.

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
  

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>