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

Message ID PR3PR06MB688962588B9E9A50636E7334FF2A2@PR3PR06MB6889.eurprd06.prod.outlook.com (mailing list archive)
State New
Headers
Series nptl: futex_lock_pi deadlock detection provides valuable information but it is turned into a rather cryptic assertion failure |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686

Commit Message

Moritz KLAMMLER (FERCHAU) April 23, 2026, 7:50 p.m. UTC
  Here is an updated version of the patch, as per the latest discussion
regarding the behavior of recursive mutexes.

Besides rewording the commit message and commentary, the only logical change
compared to the previous version is replacing

    if (e == EDEADLK
        && (kind == PTHREAD_MUTEX_ERRORCHECK_NP
        || kind == PTHREAD_MUTEX_RECURSIVE_NP))

in nptl/pthread_mutex_lock.c with

    if (e == EDEADLK && kind == PTHREAD_MUTEX_ERRORCHECK_NP)

and changing

    return (that->prio_inherit)
        && (that->type == PTHREAD_MUTEX_ERRORCHECK
        || that->type == PTHREAD_MUTEX_RECURSIVE);

in nptl/tst-deadlk.c to

    return that->prio_inherit && that->type == PTHREAD_MUTEX_ERRORCHECK;

accordingly.  Below is the complete, updated patch.

KR Moritz



From 934ad1fbf3bd9ac3a3d7b66cf10e8e6d0afa3b48 Mon Sep 17 00:00:00 2001
From: Moritz Klammler <moritz.klammler.ext@siemens.com>
Date: Thu, 23 Apr 2026 21:31:55 +0200
Subject: [PATCH 1/1] nptl: Propagate EDEADLK from FUTEX_LOCK_PI for
 errror-checking mutexes

This patch changes the behavior of pthread_mutex_lock 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:

 - For error-checking PI mutexes; 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.  Since
   error-checking mutexes are specified to possibly return EDEADLK,
   and the only reason to use them in the first place is for the sake
   of these additional error checks, any calling code failing to check
   the return code in this case may legitimately be considered broken
   already.

 - For recursive mutexes; the thread will actually deadlock instead of
   failing the assertion.  It has been discussed (see below) that this
   might be more conservative as, unfortunately, lots of existing code
   might be guilty of not always checking the return code.  So
   returning at all in this case might cause (arguably questionable)
   code to continue executing undefined behavior by falsely assuming
   that the thread successfully acquired a lock, which it didn't.

 - For all other mutex types; the behavior is not changed.  They will
   continue to actually deadlock the calling thread as they did prior
   to this patch.

A new test is added to assert the expected behavior of all mutex
types.

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) discussions can be seen here:
https://sourceware.org/pipermail/libc-alpha/2025-December/173431.html
https://sourceware.org/pipermail/libc-alpha/2026-April/176406.html

Signed-off-by: Moritz Klammler <moritz.klammler.ext@siemens.com>
---
 nptl/Makefile             |   1 +
 nptl/pthread_mutex_lock.c |  13 +++-
 nptl/tst-deadlk.c         | 144 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+), 3 deletions(-)
 create mode 100644 nptl/tst-deadlk.c
  

Patch

diff --git a/nptl/Makefile b/nptl/Makefile
index 08b8ba8a31..eb621d0b40 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -282,6 +282,7 @@  tests = \
   tst-cleanup5 \
   tst-cond26 \
   tst-context1 \
+  tst-deadlk \
   tst-default-attr \
   tst-dlsym1 \
   tst-exec4 \
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index a697f2b6ca..dfaf46d5fc 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -418,9 +418,16 @@  __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)
+		  {
+		    /* FUTEX_LOCK_PI may return EDEADLK due to cross-thread
+		       deadlock detection, beyond the same-thread recursive
+		       check above.  Pass this error through for error-checking
+		       mutexes; otherwise, intentionally deadlock for all other
+		       mutex types.  */
+		    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.c b/nptl/tst-deadlk.c
new file mode 100644
index 0000000000..bf787826ae
--- /dev/null
+++ b/nptl/tst-deadlk.c
@@ -0,0 +1,144 @@ 
+/* Test for pthread_mutex_lock deadlock behavior.
+   Copyright (C) 2026 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <array_length.h>
+#include <errno.h>
+#include <stdint.h>
+#include <stdio.h>
+
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/test-driver.h>
+#include <support/xthread.h>
+
+#define ASSUME_DEADLOCK_AFTER_SECONDS 3
+
+struct which_mutex
+{
+  int type;
+  bool prio_inherit;
+  bool robust;
+};
+
+struct task_context
+{
+  pthread_mutex_t *first, *second;
+  pthread_barrier_t *barrier;
+};
+
+static bool
+should_detect_deadlock (const struct which_mutex *const that)
+{
+  return that->prio_inherit && that->type == PTHREAD_MUTEX_ERRORCHECK;
+}
+
+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
+prepare_mutex (pthread_mutex_t *const mutex,
+               const struct which_mutex *const that)
+{
+  pthread_mutexattr_t attr;
+  xpthread_mutexattr_init (&attr);
+  xpthread_mutexattr_settype (&attr, that->type);
+  if (that->robust)
+    xpthread_mutexattr_setrobust (&attr, PTHREAD_MUTEX_ROBUST);
+  if (that->prio_inherit)
+    xpthread_mutexattr_setprotocol (&attr, PTHREAD_PRIO_INHERIT);
+  xpthread_mutex_init (mutex, &attr);
+  xpthread_mutexattr_destroy (&attr);
+}
+
+static void
+do_test_single (void *const ctx)
+{
+  pthread_mutex_t m1, m2;
+  pthread_barrier_t barrier;
+  const struct which_mutex *const that = ctx;
+  const bool graceful = should_detect_deadlock (that);
+  struct task_context ctx1
+      = { .first = &m1, .second = &m2, .barrier = &barrier };
+  struct task_context ctx2
+      = { .first = &m2, .second = &m1, .barrier = &barrier };
+  xpthread_barrier_init (&barrier, NULL, 2);
+  prepare_mutex (&m1, that);
+  prepare_mutex (&m2, that);
+  const pthread_t t1 = xpthread_create (NULL, thread_function, &ctx1);
+  const pthread_t t2 = xpthread_create (NULL, thread_function, &ctx2);
+  if (!graceful)
+    delayed_exit (ASSUME_DEADLOCK_AFTER_SECONDS);
+  const int ret1 = (intptr_t) xpthread_join (t1);
+  const int ret2 = (intptr_t) xpthread_join (t2);
+  xpthread_mutex_destroy (&m1);
+  xpthread_mutex_destroy (&m2);
+  xpthread_barrier_destroy (&barrier);
+  TEST_VERIFY (graceful);
+  TEST_VERIFY (ret1 == 0 || ret1 == EDEADLK);
+  TEST_VERIFY (ret2 == 0 || ret2 == EDEADLK);
+  TEST_VERIFY (ret1 == EDEADLK || ret2 == EDEADLK);
+}
+
+static int
+do_test (void)
+{
+  const int mutex_types[] = {
+    PTHREAD_MUTEX_TIMED_NP,
+    PTHREAD_MUTEX_RECURSIVE_NP,
+    PTHREAD_MUTEX_ERRORCHECK_NP,
+    PTHREAD_MUTEX_ADAPTIVE_NP,
+  };
+  for (size_t i = 0; i < array_length (mutex_types); ++i)
+    {
+      for (int pi = 0; pi < 2; ++pi)
+        {
+          for (int rb = 0; rb < 2; ++rb)
+            {
+              struct which_mutex that = {
+                .type = mutex_types[i],
+                .prio_inherit = pi,
+                .robust = rb,
+              };
+              const char *const description
+                  = xasprintf ("type = %d, prio_inherit = %d, robust = %d",
+                               that.type, that.robust, that.prio_inherit);
+              struct support_capture_subprocess capture
+                  = support_capture_subprocess (do_test_single, &that);
+              support_capture_subprocess_check (&capture, description, 0,
+                                                sc_allow_none);
+              support_capture_subprocess_free (&capture);
+            }
+        }
+    }
+  return 0;
+}
+
+#define TIMEOUT 60
+#include <support/test-driver.c>