[PATCH] nptl: Propagate kernel EDEADLK and clear robust list on PI mutex deadlock
Adhemerval Zanella <[email protected]>
| Newsgroups | gmane.comp.lib.glibc.alpha |
|---|---|
| Message-ID | <[email protected]> |
The kernel FUTEX_LOCK_PI deadlock detection is handled
inconsistently. The pthread_mutex_lock passes EDEADLK through for
error-checking mutexes without clearing the pending robust list
operation (set for robust mutexes, allowing the same corruption at
thread exit as bug 34542), while pthread_mutex_timedlock still
asserted the error could not happen and aborted.
This was a missing spot add by 0e30e9ce27 not handled by 69841bbaa5.
Also tuned down tst-deadlk.c timers to avoid it taking too much time.
With the inclusion on the new tests total time was about 42s on
recent x86_64 box, it is not around 9s.
Checked on x86_64-linux-gnu.
---
nptl/pthread_mutex_lock.c | 2 +-
nptl/pthread_mutex_timedlock.c | 14 +---
nptl/tst-deadlk.c | 92 ++++++++++++++++-----
nptl/tst-robust12.c | 144 +++++++++++++++++++++++++++++++++
sysdeps/nptl/pthreadP.h | 7 ++
5 files changed, 226 insertions(+), 33 deletions(-)
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 3e82eccf3b..0603fc165c 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -425,7 +425,7 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex)
check above. Pass this error through for error-checking
mutexes; otherwise, intentionally deadlock for all other
mutex types. */
- return e;
+ return __pthread_mutex_robust_error (e);
}
/* ESRCH can happen only for non-robust PI mutexes where
diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index 2a44108736..0bcf327fd3 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -28,14 +28,6 @@
#include <stap-probe.h>
-/* Return the error code ERR after clearing the robust list head. */
-static int
-__pthread_mutex_robust_error (int err)
-{
- THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
- return err;
-}
-
int
__pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
clockid_t clockid,
@@ -352,9 +344,9 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
return __pthread_mutex_robust_error (ETIMEDOUT);
else 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)
+ return __pthread_mutex_robust_error (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
index bf787826ae..913dd62810 100644
--- a/nptl/tst-deadlk.c
+++ b/nptl/tst-deadlk.c
@@ -25,7 +25,9 @@
#include <support/check.h>
#include <support/support.h>
#include <support/test-driver.h>
+#include <support/timespec.h>
#include <support/xthread.h>
+#include <support/xtime.h>
#define ASSUME_DEADLOCK_AFTER_SECONDS 3
@@ -34,12 +36,16 @@ struct which_mutex
int type;
bool prio_inherit;
bool robust;
+ bool timed;
};
struct task_context
{
pthread_mutex_t *first, *second;
pthread_barrier_t *barrier;
+ bool timed;
+ /* Relative timeout for the timed variant. */
+ struct timespec timeout;
};
static bool
@@ -55,7 +61,17 @@ thread_function (void *const arg)
intptr_t ret = 0;
xpthread_mutex_lock (ctx->first);
xpthread_barrier_wait (ctx->barrier);
- ret = pthread_mutex_lock (ctx->second);
+ if (ctx->timed)
+ {
+ /* The timeout is not expected to be reached: the kernel either
+ reports the deadlock right away or grants the lock once the
+ other thread got EDEADLK and released it. */
+ struct timespec t = timespec_add (xclock_now (CLOCK_REALTIME),
+ ctx->timeout);
+ ret = pthread_mutex_timedlock (ctx->second, &t);
+ }
+ else
+ ret = pthread_mutex_lock (ctx->second);
xpthread_mutex_unlock (ctx->first);
if (ret == 0)
xpthread_mutex_unlock (ctx->second);
@@ -84,26 +100,41 @@ do_test_single (void *const ctx)
pthread_barrier_t barrier;
const struct which_mutex *const that = ctx;
const bool graceful = should_detect_deadlock (that);
+ const struct timespec timeout = graceful
+ ? make_timespec (5, 0) : make_timespec (0, 200000000);
struct task_context ctx1
- = { .first = &m1, .second = &m2, .barrier = &barrier };
+ = { .first = &m1, .second = &m2, .barrier = &barrier,
+ .timed = that->timed, .timeout = timeout };
struct task_context ctx2
- = { .first = &m2, .second = &m1, .barrier = &barrier };
+ = { .first = &m2, .second = &m1, .barrier = &barrier,
+ .timed = that->timed, .timeout = timeout };
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)
+ if (!graceful && !that->timed)
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);
+ if (that->timed && !graceful)
+ {
+ /* The deadlock is not detected: the lock attempts block until the
+ timeout expires. */
+ TEST_VERIFY (ret1 == 0 || ret1 == ETIMEDOUT);
+ TEST_VERIFY (ret2 == 0 || ret2 == ETIMEDOUT);
+ TEST_VERIFY (ret1 == ETIMEDOUT || ret2 == ETIMEDOUT);
+ }
+ else
+ {
+ TEST_VERIFY (graceful);
+ TEST_VERIFY (ret1 == 0 || ret1 == EDEADLK);
+ TEST_VERIFY (ret2 == 0 || ret2 == EDEADLK);
+ TEST_VERIFY (ret1 == EDEADLK || ret2 == EDEADLK);
+ }
}
static int
@@ -121,19 +152,38 @@ do_test (void)
{
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);
+ for (int timed = 0; timed < 2; ++timed)
+ {
+ struct which_mutex that = {
+ .type = mutex_types[i],
+ .prio_inherit = pi,
+ .robust = rb,
+ .timed = timed,
+ };
+ /* The timed variant covers all combinations: where the
+ kernel does not detect the deadlock, expiring the
+ timeout is the expected way out. The plain
+ pthread_mutex_lock variant genuinely deadlocks in
+ those combinations and costs
+ ASSUME_DEADLOCK_AFTER_SECONDS each, so run it only
+ where the deadlock is detected, plus one
+ representative deadlocking combination per PI
+ setting. */
+ if (!timed && !should_detect_deadlock (&that)
+ && !(that.type == PTHREAD_MUTEX_TIMED_NP
+ && !that.robust))
+ continue;
+ const char *const description
+ = xasprintf ("type = %d, prio_inherit = %d, "
+ "robust = %d, timed = %d",
+ that.type, that.prio_inherit, that.robust,
+ that.timed);
+ 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);
+ }
}
}
}
diff --git a/nptl/tst-robust12.c b/nptl/tst-robust12.c
index 0da0a358af..0f981ac4ec 100644
--- a/nptl/tst-robust12.c
+++ b/nptl/tst-robust12.c
@@ -148,6 +148,142 @@ test_one (bool use_pi)
xpthread_barrier_destroy (&barrier);
}
+
+/* Check if the pthread clear pending robut list for the kernel detected
+ deadlock cases. */
+
+static pthread_mutex_t mutex2;
+
+/* Fro the kABI futex interface. */
+#define FUTEX_WAITERS 0x80000000
+
+struct deadlock_args
+{
+ bool use_timedlock;
+ int result;
+};
+
+static void *
+deadlock_threadfunc (void *closure)
+{
+ struct deadlock_args *args = closure;
+
+ xpthread_mutex_lock (&mutex);
+ futex_value = mutex.__data.__lock;
+ xpthread_mutex_unlock (&mutex);
+ xpthread_barrier_wait (&barrier);
+
+ xpthread_barrier_wait (&barrier);
+ xpthread_mutex_lock (&mutex2);
+ xpthread_barrier_wait (&barrier);
+
+ while ((__atomic_load_n (&mutex2.__data.__lock, __ATOMIC_RELAXED)
+ & FUTEX_WAITERS) == 0)
+ nanosleep (&(struct timespec) { 0, 100000 }, NULL);
+
+ /* For the case where the kernel has not yet see the main thread as blocked
+ (schedule pressure, system load, main thread has timed out, etc), the
+ deadload is not reported and the lock attempt succeeds. So retry for
+ such cases. */
+ int ret;
+ if (args->use_timedlock)
+ {
+ /* The timeout is not expected to be triggered, the kernel should
+ report the deadlock. */
+ struct timespec t = timespec_add (xclock_now (CLOCK_REALTIME),
+ make_timespec (4, 0));
+ ret = pthread_mutex_timedlock (&mutex, &t);
+ }
+ else
+ ret = pthread_mutex_lock (&mutex);
+ args->result = ret;
+ if (ret == 0)
+ xpthread_mutex_unlock (&mutex);
+
+ xpthread_barrier_wait (&barrier);
+ xpthread_barrier_wait (&barrier);
+
+ return NULL;
+}
+
+enum { no_deadloadk_attempts = 16 };
+
+static void
+test_deadlock (bool use_timedlock)
+{
+ struct deadlock_args args = { .use_timedlock = use_timedlock };
+
+ /* Retry in the case the deadlock is not reported to the test thread. */
+ for (int attempt = 0; attempt < no_deadloadk_attempts; ++attempt)
+ {
+ xpthread_barrier_init (&barrier, NULL, 2);
+
+ {
+ pthread_mutexattr_t a;
+ xpthread_mutexattr_init (&a);
+ TEST_COMPARE (pthread_mutexattr_setrobust (&a, PTHREAD_MUTEX_ROBUST),
+ 0);
+ /* NB: error checkin, so that kernel detects the deadlock. */
+ xpthread_mutexattr_settype (&a, PTHREAD_MUTEX_ERRORCHECK);
+ xpthread_mutexattr_setprotocol (&a, PTHREAD_PRIO_INHERIT);
+ xpthread_mutex_init (&mutex, &a);
+ xpthread_mutex_init (&mutex2, &a);
+ xpthread_mutexattr_destroy (&a);
+ }
+
+ args.result = -1;
+ pthread_t thr = xpthread_create (NULL, deadlock_threadfunc, &args);
+
+ xpthread_barrier_wait (&barrier);
+
+ xpthread_mutex_lock (&mutex);
+ /* Allow the test thread to lock MUTEX2. */
+ xpthread_barrier_wait (&barrier);
+ /* Wait for the test thread to lock MUTEX2. */
+ xpthread_barrier_wait (&barrier);
+
+ struct timespec t = timespec_add (xclock_now (CLOCK_REALTIME),
+ make_timespec (2, 0));
+ int ret = pthread_mutex_timedlock (&mutex2, &t);
+ TEST_VERIFY (ret == ETIMEDOUT || ret == EDEADLK);
+
+ /* In the expected case the target should have received EDEADLK,
+ otherwise the kernel reported the deadlock to this thread instead,
+ or the timeout expired efore thre test thread attempted the lock.
+ For the later retry the round. */
+ xpthread_mutex_unlock (&mutex);
+
+ /* Wait for the test thread's lock attempt to finish. */
+ xpthread_barrier_wait (&barrier);
+
+ if (args.result != 0)
+ TEST_COMPARE (args.result, EDEADLK);
+
+ /* Destroy MUTEX and overwrite it with a different bit pattern than a
+ valid futex value that will be acted upon by the kernel if the
+ robust list of the exiting test thread still references the mutex.
+ MUTEX2 is still owned by the test thread and is released by the
+ kernel when the thread exits. */
+ xpthread_mutex_destroy (&mutex);
+ char pattern[sizeof (mutex)];
+ memset (pattern, 0xcc, sizeof (pattern));
+ memcpy (&pattern[offsetof (pthread_mutex_t, __data.__lock)],
+ &futex_value, sizeof (futex_value));
+ memcpy (&mutex, pattern, sizeof (mutex));
+
+ xpthread_barrier_wait (&barrier);
+ xpthread_join (thr);
+
+ TEST_COMPARE_BLOB (&mutex, sizeof (mutex), pattern, sizeof (pattern));
+
+ xpthread_barrier_destroy (&barrier);
+
+ if (args.result == EDEADLK)
+ return;
+ }
+ FAIL_EXIT1 ("deadlock was never reported to the test thread");
+}
+
static int
do_test (void)
{
@@ -160,6 +296,14 @@ do_test (void)
test_one (use_pi);
}
+ for (int use_timedlock = 0; use_timedlock < 2; ++use_timedlock)
+ {
+ printf ("info: deadlock subtest, %s\n",
+ use_timedlock ? "pthread_mutex_timedlock"
+ : "pthread_mutex_lock");
+ test_deadlock (use_timedlock);
+ }
+
return 0;
}
diff --git a/sysdeps/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h
index 982705aee5..bfe184a9ca 100644
--- a/sysdeps/nptl/pthreadP.h
+++ b/sysdeps/nptl/pthreadP.h
@@ -214,6 +214,13 @@ libc_hidden_proto (__pthread_current_priority)
/* If OLDPRIO is not -1, undo its TPP priority change. Return ERR. */
int __pthread_mutex_priority_error (int err, int oldprio) attribute_hidden;
+static __always_inline int
+__pthread_mutex_robust_error (int err)
+{
+ THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
+ return err;
+}
+
/* This will not catch all invalid descriptors but is better than
nothing. And if the test triggers the thread descriptor is
--
2.53.0