[PATCH] nptl: Fix __futex_clocklock64 return error check [BZ #26964]

["Lucas A. M. Magalhaes" <lamm@linux.ibm.com>]

The earlier implementation of this, __lll_clocklock, calls lll_clockwait
that doesn't return the futex syscall error codes.  It always tries again
if that fails.

However in the current implementation, when the futex returns EAGAIN,
__futex_clocklock64 will also return EGAIN, even if the futex is taken.

This patch fixes the EAGAIN issue and also adds a check for EINTR.  As
futex syscall can return EINTR if the thread is interrupted by a signal.
In this case I'm assuming the function should continue trying to lock as
there is no mention to about it on POSIX.  Also add a test for both
scenarios.
---
 nptl/Makefile                         |   2 +-
 nptl/tst-pthread-timedlock-lockloop.c | 136 ++++++++++++++++++++++++++
 sysdeps/nptl/futex-internal.h         |   9 ++
 3 files changed, 146 insertions(+), 1 deletion(-)
 create mode 100644 nptl/tst-pthread-timedlock-lockloop.c

diff --git a/nptl/Makefile b/nptl/Makefile
index a48426a396..91324e09f2 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -298,7 +298,7 @@ tests = tst-attr2 tst-attr3 tst-default-attr \
 	tst-thread-affinity-sched \
 	tst-pthread-defaultattr-free \
 	tst-pthread-attr-sigmask \
-
+	tst-pthread-timedlock-lockloop \
 
 tests-container =  tst-pthread-getattr
 
diff --git a/nptl/tst-pthread-timedlock-lockloop.c b/nptl/tst-pthread-timedlock-lockloop.c
new file mode 100644
index 0000000000..abcc5724cf
--- /dev/null
+++ b/nptl/tst-pthread-timedlock-lockloop.c
@@ -0,0 +1,136 @@
+/* Make sure pthrea_mutex_timedlock doesn't return spurious error codes.
+
+   Copyright (C) 2020 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 <errno.h>
+#include <pthread.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+
+#define NANO_PER_SEC 1000000000LL
+#define TIMEOUT (NANO_PER_SEC / 1000LL)
+#define NUM_THREADS 50
+#define RETEST_TIMES 100
+
+struct worker_args
+{
+  pthread_mutex_t mtx;
+  int runs;
+};
+
+void
+signal_handler (int sig_num)
+{
+  if (sig_num != SIGUSR1)
+    printf ("Unexpected signal");
+}
+
+/* Call pthread_mutex_timedlock()/pthread_mutex_unlock() repetitively, hoping
+   that one of them returns EAGAIN or EINTR unexpectedly.  */
+static void *
+worker (void *arg)
+{
+  pthread_mutex_t *mtx = &(((struct worker_args *) arg)->mtx);
+  struct timespec abs_time;
+  signal (SIGUSR1, signal_handler);
+
+  for (unsigned run = 0; run < ((struct worker_args *) arg)->runs; run++)
+    {
+      clock_gettime (CLOCK_REALTIME, &abs_time);
+      abs_time.tv_nsec += TIMEOUT;
+      if (abs_time.tv_nsec >= NANO_PER_SEC)
+	{
+	  abs_time.tv_sec++;
+	  abs_time.tv_nsec -= NANO_PER_SEC;
+	}
+
+      int ret = pthread_mutex_timedlock (mtx, &abs_time);
+
+      if (ret == 0)
+	pthread_mutex_unlock (mtx);
+
+      if (ret == EAGAIN || ret == EINTR)
+	{
+	  printf ("Unexpected return %d\n", ret);
+	  return (void *) 1;
+	}
+    }
+  return NULL;
+}
+
+static int
+do_test (void)
+{
+  pthread_t *workers =
+    (pthread_t *) malloc (NUM_THREADS * sizeof (pthread_t));
+
+  struct worker_args args;
+  pthread_mutex_init (&(args.mtx), NULL);
+
+  int *thread_ret = NULL, ret = 0;
+
+  /* Run the checks to catch an EAGAIN.  */
+  /* As there is no way to ensure the error condition, just run the test many
+     times hoping to catch the error.  */
+  args.runs = 100;
+  for (int run = 0; run < RETEST_TIMES; run++)
+    {
+      for (int i = 0; i < NUM_THREADS; i++)
+	{
+	  pthread_create (&workers[i], NULL, worker, (void *) &args);
+	}
+      for (int i = 0; i < NUM_THREADS; i++)
+	{
+	  pthread_join (workers[i], (void **) &thread_ret);
+	  if (thread_ret != NULL)
+	    ++ret;
+	}
+      if (ret != 0)
+	goto err;
+    }
+
+  /* Run the test to check if we catch an EINTR.  */
+  /* As there is no way to ensure the error condition, just run the test many
+     times hoping to catch the error.  */
+  pthread_t thread;
+  args.runs = 1;
+  for (int i = 0; i < RETEST_TIMES * 1000; i++)
+    {
+      if (pthread_mutex_lock (&(args.mtx)) != 0)
+	{
+	  printf ("Mutex lock failed\n");
+	  goto err;
+	}
+      pthread_create (&thread, NULL, worker, (void *) &args);
+      pthread_kill (thread, SIGUSR1);
+      pthread_mutex_unlock (&(args.mtx));
+      pthread_join (thread, (void **) &thread_ret);
+      if (thread_ret != NULL)
+	goto err;
+    }
+
+  free (workers);
+  return 0;
+
+err:
+  free (workers);
+  return 1;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index e67803952f..f16d26d994 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -424,10 +424,19 @@ __futex_clocklock64 (int *futex, clockid_t clockid,
     {
       while (atomic_exchange_acq (futex, 2) != 0)
         {
+	  /* At this point we tried to get the futex but failed and set its
+	     value to 2.  However the futex value can be changed by other
+	     thread before this calls the futex syscall.  If so the syscall
+	     will return EAGAIN.  */
           err = __futex_abstimed_wait64 ((unsigned int *) futex, 2, clockid,
 					 abstime, private);
           if (err == EINVAL || err == ETIMEDOUT || err == EOVERFLOW)
             break;
+	  /* If EAGAIN or EINTR is returned here the error code should be reset
+	     as we will try again to acquire the futex and it may success.
+	     Otherwise the mutex will be locked and the return will not be 0.  */
+	  if (err == EAGAIN || err == EINTR)
+	    err = 0;
         }
     }
   return err;
-- 
2.26.2