]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
nptl: Consolidate pthread_{timed,try}join{_np}
authorAdhemerval Zanella <adhemerval.zanella@linaro.org>
Wed, 25 Jan 2017 19:08:51 +0000 (17:08 -0200)
committerAdhemerval Zanella <adhemerval.zanella@linaro.org>
Wed, 20 Dec 2017 13:32:54 +0000 (11:32 -0200)
This patch consolidates the pthread_join and gnu extensions to avoid
code duplication.  The function pthread_join, pthread_tryjoin_np, and
pthread_timedjoin_np are now based on pthread_timedjoin_ex.

It also fixes some inconsistencies on ESRCH, EINVAL, EDEADLK handling
(where each implementation differs from each other) and also on
clenup handler (which now always use a CAS).

Checked on i686-linux-gnu and x86_64-linux-gnu.

* nptl/pthreadP.h (__pthread_timedjoin_np): Define.
* nptl/pthread_join.c (pthread_join): Use __pthread_timedjoin_np.
* nptl/pthread_tryjoin.c (pthread_tryjoin): Likewise.
* nptl/pthread_timedjoin.c (cleanup): Use CAS on argument setting.
(pthread_timedjoin_np): Define internal symbol and common code from
pthread_join.
* sysdeps/unix/sysv/linux/i386/lowlevellock.h (__lll_timedwait_tid):
Remove superflous checks.
* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (__lll_timedwait_tid):
Likewise.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
ChangeLog
nptl/Makefile
nptl/pthreadP.h
nptl/pthread_join.c
nptl/pthread_join_common.c [new file with mode: 0644]
nptl/pthread_timedjoin.c
nptl/pthread_tryjoin.c
sysdeps/unix/sysv/linux/i386/lowlevellock.h
sysdeps/unix/sysv/linux/x86_64/lowlevellock.h

index ad2c9db353be4b81047d573519614ecf70830b3c..35d1d102f6a0882b58396a1cb6177edbeb089417 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2017-12-20  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
+
+       * nptl/Makefile (libpthread-routines): Add pthread_join_common.
+       * nptl/pthreadP.h (__pthread_timedjoin_ex): New prototype.
+       * nptl/pthread_join_common.c: New file: common function used on
+       pthread_join, pthread_timedjoin_np, pthread_tryjoin_np.
+       * nptl/pthread_join.c (pthread_join): Use __pthread_timedjoin_ex.
+       * nptl/pthread_tryjoin.c (pthread_tryjoin): Likewise.
+       * nptl/pthread_timedjoin.c (pthread_timedjoin_np): Likewise.
+       (cleanup): Move definition to pthread_join_common.c.
+       * sysdeps/unix/sysv/linux/i386/lowlevellock.h (__lll_timedwait_tid):
+       Remove superflous checks.
+       * sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (__lll_timedwait_tid):
+       Likewise.
+
 2017-12-20  Szabolcs Nagy  <szabolcs.nagy@arm.com>
 
        * sysdeps/aarch64/libm-test-ulps: Update.
index ab8cc98728ed27d28646f333bb5229ffdc463c65..cf2ba8131b72d271ad5515556bec0920c4c9841d 100644 (file)
@@ -49,6 +49,7 @@ pthread-compat-wrappers = \
 libpthread-routines = nptl-init vars events version pt-interp \
                      pthread_create pthread_exit pthread_detach \
                      pthread_join pthread_tryjoin pthread_timedjoin \
+                     pthread_join_common \
                      compat-pthread_self pthread_equal pthread_yield \
                      pthread_getconcurrency pthread_setconcurrency \
                      pthread_getschedparam pthread_setschedparam \
index 713000e32d470ad1bfde34e1d83b5e06f341dbf1..2646b8f605abd2984c451499ab6969454185d764 100644 (file)
@@ -506,6 +506,8 @@ extern int __pthread_setcanceltype (int type, int *oldtype);
 extern int __pthread_enable_asynccancel (void) attribute_hidden;
 extern void __pthread_disable_asynccancel (int oldtype) attribute_hidden;
 extern void __pthread_testcancel (void);
+extern int __pthread_timedjoin_ex (pthread_t, void **, const struct timespec *,
+                                  bool);
 
 #if IS_IN (libpthread)
 hidden_proto (__pthread_mutex_init)
@@ -524,6 +526,7 @@ hidden_proto (__pthread_setcancelstate)
 hidden_proto (__pthread_testcancel)
 hidden_proto (__pthread_mutexattr_init)
 hidden_proto (__pthread_mutexattr_settype)
+hidden_proto (__pthread_timedjoin_ex)
 #endif
 
 extern int __pthread_cond_broadcast_2_0 (pthread_cond_2_0_t *cond);
index afc8c379cad3f537da4e57606585a2826270dd55..47379a67a36c877ba8582bb180827c4092b441d4 100644 (file)
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <stdlib.h>
-
-#include <atomic.h>
 #include "pthreadP.h"
 
-#include <stap-probe.h>
-
-
-static void
-cleanup (void *arg)
-{
-  /* If we already changed the waiter ID, reset it.  The call cannot
-     fail for any reason but the thread not having done that yet so
-     there is no reason for a loop.  */
-  (void) atomic_compare_and_exchange_bool_acq ((struct pthread **) arg, NULL,
-                                              THREAD_SELF);
-}
-
-
 int
 __pthread_join (pthread_t threadid, void **thread_return)
 {
-  struct pthread *pd = (struct pthread *) threadid;
-
-  /* Make sure the descriptor is valid.  */
-  if (INVALID_NOT_TERMINATED_TD_P (pd))
-    /* Not a valid thread handle.  */
-    return ESRCH;
-
-  /* Is the thread joinable?.  */
-  if (IS_DETACHED (pd))
-    /* We cannot wait for the thread.  */
-    return EINVAL;
-
-  struct pthread *self = THREAD_SELF;
-  int result = 0;
-
-  LIBC_PROBE (pthread_join, 1, threadid);
-
-  /* During the wait we change to asynchronous cancellation.  If we
-     are canceled the thread we are waiting for must be marked as
-     un-wait-ed for again.  */
-  pthread_cleanup_push (cleanup, &pd->joinid);
-
-  /* Switch to asynchronous cancellation.  */
-  int oldtype = CANCEL_ASYNC ();
-
-  if ((pd == self
-       || (self->joinid == pd
-          && (pd->cancelhandling
-              & (CANCELING_BITMASK | CANCELED_BITMASK | EXITING_BITMASK
-                 | TERMINATED_BITMASK)) == 0))
-      && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling))
-    /* This is a deadlock situation.  The threads are waiting for each
-       other to finish.  Note that this is a "may" error.  To be 100%
-       sure we catch this error we would have to lock the data
-       structures but it is not necessary.  In the unlikely case that
-       two threads are really caught in this situation they will
-       deadlock.  It is the programmer's problem to figure this
-       out.  */
-    result = EDEADLK;
-  /* Wait for the thread to finish.  If it is already locked something
-     is wrong.  There can only be one waiter.  */
-  else if (__builtin_expect (atomic_compare_and_exchange_bool_acq (&pd->joinid,
-                                                                  self,
-                                                                  NULL), 0))
-    /* There is already somebody waiting for the thread.  */
-    result = EINVAL;
-  else
-    /* Wait for the child.  */
-    lll_wait_tid (pd->tid);
-
-
-  /* Restore cancellation mode.  */
-  CANCEL_RESET (oldtype);
-
-  /* Remove the handler.  */
-  pthread_cleanup_pop (0);
-
-
-  if (__glibc_likely (result == 0))
-    {
-      /* We mark the thread as terminated and as joined.  */
-      pd->tid = -1;
-
-      /* Store the return value if the caller is interested.  */
-      if (thread_return != NULL)
-       *thread_return = pd->result;
-
-
-      /* Free the TCB.  */
-      __free_tcb (pd);
-    }
-
-  LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result);
-
-  return result;
+  return __pthread_timedjoin_ex (threadid, thread_return, NULL, true);
 }
 weak_alias (__pthread_join, pthread_join)
diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
new file mode 100644 (file)
index 0000000..59c5445
--- /dev/null
@@ -0,0 +1,115 @@
+/* Common definition for pthread_{timed,try}join{_np}.
+   Copyright (C) 2017 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include "pthreadP.h"
+#include <atomic.h>
+#include <stap-probe.h>
+
+static void
+cleanup (void *arg)
+{
+  /* If we already changed the waiter ID, reset it.  The call cannot
+     fail for any reason but the thread not having done that yet so
+     there is no reason for a loop.  */
+  struct pthread *self = THREAD_SELF;
+  atomic_compare_exchange_weak_acquire (&arg, &self, NULL);
+}
+
+int
+__pthread_timedjoin_ex (pthread_t threadid, void **thread_return,
+                       const struct timespec *abstime, bool block)
+{
+  struct pthread *pd = (struct pthread *) threadid;
+
+  /* Make sure the descriptor is valid.  */
+  if (INVALID_NOT_TERMINATED_TD_P (pd))
+    /* Not a valid thread handle.  */
+    return ESRCH;
+
+  /* Is the thread joinable?.  */
+  if (IS_DETACHED (pd))
+    /* We cannot wait for the thread.  */
+    return EINVAL;
+
+  struct pthread *self = THREAD_SELF;
+  int result = 0;
+
+  LIBC_PROBE (pthread_join, 1, threadid);
+
+  if ((pd == self
+       || (self->joinid == pd
+          && (pd->cancelhandling
+              & (CANCELING_BITMASK | CANCELED_BITMASK | EXITING_BITMASK
+                 | TERMINATED_BITMASK)) == 0))
+      && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling))
+    /* This is a deadlock situation.  The threads are waiting for each
+       other to finish.  Note that this is a "may" error.  To be 100%
+       sure we catch this error we would have to lock the data
+       structures but it is not necessary.  In the unlikely case that
+       two threads are really caught in this situation they will
+       deadlock.  It is the programmer's problem to figure this
+       out.  */
+    return EDEADLK;
+
+  /* Wait for the thread to finish.  If it is already locked something
+     is wrong.  There can only be one waiter.  */
+  else if (__glibc_unlikely (atomic_compare_exchange_weak_acquire (&pd->joinid,
+                                                                  &self,
+                                                                  NULL)))
+    /* There is already somebody waiting for the thread.  */
+    return EINVAL;
+
+  if (block)
+    {
+      /* During the wait we change to asynchronous cancellation.  If we
+        are cancelled the thread we are waiting for must be marked as
+        un-wait-ed for again.  */
+      pthread_cleanup_push (cleanup, &pd->joinid);
+
+      int oldtype = CANCEL_ASYNC ();
+
+      if (abstime != NULL)
+       result = lll_timedwait_tid (pd->tid, abstime);
+      else
+       lll_wait_tid (pd->tid);
+
+      CANCEL_RESET (oldtype);
+
+      pthread_cleanup_pop (0);
+    }
+
+  if (__glibc_likely (result == 0))
+    {
+      /* We mark the thread as terminated and as joined.  */
+      pd->tid = -1;
+
+      /* Store the return value if the caller is interested.  */
+      if (thread_return != NULL)
+       *thread_return = pd->result;
+
+      /* Free the TCB.  */
+      __free_tcb (pd);
+    }
+  else
+    pd->joinid = NULL;
+
+  LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result);
+
+  return result;
+}
+hidden_def (__pthread_timedjoin_ex)
index 567c171133c6aa94e910955fca4ba89da0983c72..0f74a98c203353af258b2cf15937cbbb7febe9fd 100644 (file)
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <stdlib.h>
-#include <atomic.h>
 #include "pthreadP.h"
 
-
-static void
-cleanup (void *arg)
-{
-  *(void **) arg = NULL;
-}
-
-
 int
-pthread_timedjoin_np (pthread_t threadid, void **thread_return,
-                     const struct timespec *abstime)
+__pthread_timedjoin_np (pthread_t threadid, void **thread_return,
+                       const struct timespec *abstime)
 {
-  struct pthread *self;
-  struct pthread *pd = (struct pthread *) threadid;
-  int result;
-
-  /* Make sure the descriptor is valid.  */
-  if (INVALID_NOT_TERMINATED_TD_P (pd))
-    /* Not a valid thread handle.  */
-    return ESRCH;
-
-  /* Is the thread joinable?.  */
-  if (IS_DETACHED (pd))
-    /* We cannot wait for the thread.  */
-    return EINVAL;
-
-  self = THREAD_SELF;
-  if (pd == self || self->joinid == pd)
-    /* This is a deadlock situation.  The threads are waiting for each
-       other to finish.  Note that this is a "may" error.  To be 100%
-       sure we catch this error we would have to lock the data
-       structures but it is not necessary.  In the unlikely case that
-       two threads are really caught in this situation they will
-       deadlock.  It is the programmer's problem to figure this
-       out.  */
-    return EDEADLK;
-
-  /* Wait for the thread to finish.  If it is already locked something
-     is wrong.  There can only be one waiter.  */
-  if (__builtin_expect (atomic_compare_and_exchange_bool_acq (&pd->joinid,
-                                                             self, NULL), 0))
-    /* There is already somebody waiting for the thread.  */
-    return EINVAL;
-
-
-  /* During the wait we change to asynchronous cancellation.  If we
-     are cancelled the thread we are waiting for must be marked as
-     un-wait-ed for again.  */
-  pthread_cleanup_push (cleanup, &pd->joinid);
-
-  /* Switch to asynchronous cancellation.  */
-  int oldtype = CANCEL_ASYNC ();
-
-
-  /* Wait for the child.  */
-  result = lll_timedwait_tid (pd->tid, abstime);
-
-
-  /* Restore cancellation mode.  */
-  CANCEL_RESET (oldtype);
-
-  /* Remove the handler.  */
-  pthread_cleanup_pop (0);
-
-
-  /* We might have timed out.  */
-  if (result == 0)
-    {
-      /* Store the return value if the caller is interested.  */
-      if (thread_return != NULL)
-       *thread_return = pd->result;
-
-
-      /* Free the TCB.  */
-      __free_tcb (pd);
-    }
-  else
-    pd->joinid = NULL;
-
-  return result;
+  return __pthread_timedjoin_ex (threadid, thread_return, abstime, true);
 }
+weak_alias (__pthread_timedjoin_np, pthread_timedjoin_np)
index 6a3b62e4099369f8b37e0c60dccf00aa0a074da6..b34df549702540f3d667472bf25d65445e9a9cc4 100644 (file)
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <stdlib.h>
-
-#include <atomic.h>
 #include "pthreadP.h"
 
-
 int
 pthread_tryjoin_np (pthread_t threadid, void **thread_return)
 {
-  struct pthread *self;
-  struct pthread *pd = (struct pthread *) threadid;
-
-  /* Make sure the descriptor is valid.  */
-  if (DEBUGGING_P && __find_in_stack_list (pd) == NULL)
-    /* Not a valid thread handle.  */
-    return ESRCH;
-
-  /* Is the thread joinable?.  */
-  if (IS_DETACHED (pd))
-    /* We cannot wait for the thread.  */
-    return EINVAL;
-
-  self = THREAD_SELF;
-  if (pd == self || self->joinid == pd)
-    /* This is a deadlock situation.  The threads are waiting for each
-       other to finish.  Note that this is a "may" error.  To be 100%
-       sure we catch this error we would have to lock the data
-       structures but it is not necessary.  In the unlikely case that
-       two threads are really caught in this situation they will
-       deadlock.  It is the programmer's problem to figure this
-       out.  */
-    return EDEADLK;
-
   /* Return right away if the thread hasn't terminated yet.  */
+  struct pthread *pd = (struct pthread *) threadid;
   if (pd->tid != 0)
     return EBUSY;
 
-  /* Wait for the thread to finish.  If it is already locked something
-     is wrong.  There can only be one waiter.  */
-  if (atomic_compare_and_exchange_bool_acq (&pd->joinid, self, NULL))
-    /* There is already somebody waiting for the thread.  */
-    return EINVAL;
-
-  /* Store the return value if the caller is interested.  */
-  if (thread_return != NULL)
-    *thread_return = pd->result;
-
-
-  /* Free the TCB.  */
-  __free_tcb (pd);
-
-  return 0;
+  /* If pd->tid != 0 then lll_wait_tid will not block on futex
+     operation.  */
+  return __pthread_timedjoin_ex (threadid, thread_return, NULL, false);
 }
index 197bb1fcd92f99b2fe9f8f3aedb7eb44771ed2de..098c64bf96b96a4f00a5d6cebf808d18e5e736aa 100644 (file)
@@ -237,12 +237,7 @@ extern int __lll_timedwait_tid (int *tid, const struct timespec *abstime)
   ({                                                                         \
     int __result = 0;                                                        \
     if ((tid) != 0)                                                          \
-      {                                                                              \
-       if ((abstime)->tv_nsec < 0 || (abstime)->tv_nsec >= 1000000000)       \
-         __result = EINVAL;                                                  \
-       else                                                                  \
-         __result = __lll_timedwait_tid (&(tid), (abstime));                 \
-      }                                                                              \
+      __result = __lll_timedwait_tid (&(tid), (abstime));                    \
     __result; })
 
 
index cbf6597e005081eb5b959ef8fdcc0aba66712d8f..0b56a2eb1ca46a84bb8f1fb1c562cf1e43b8eb18 100644 (file)
@@ -249,12 +249,7 @@ extern int __lll_timedwait_tid (int *, const struct timespec *)
   ({                                                                         \
     int __result = 0;                                                        \
     if ((tid) != 0)                                                          \
-      {                                                                              \
-       if ((abstime)->tv_nsec < 0 || (abstime)->tv_nsec >= 1000000000)       \
-         __result = EINVAL;                                                  \
-       else                                                                  \
-         __result = __lll_timedwait_tid (&(tid), (abstime));                 \
-      }                                                                              \
+      __result = __lll_timedwait_tid (&(tid), (abstime));                    \
     __result; })
 
 extern int __lll_lock_elision (int *futex, short *adapt_count, int private)