[v3,01/12] nptl: Set cancellation type and state on pthread_exit (BZ #28267)

Message ID 20220531175255.1513396-2-adhemerval.zanella@linaro.org
State Superseded
Headers
Series Fix various NPTL synchronization issues |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella May 31, 2022, 5:52 p.m. UTC
  It is required by POSIX XSH 2.9.5 Thread Cancellation under the
heading Thread Cancellation Cleanup Handlers.

Checked x86_64-linux-gnu.
---
 nptl/Makefile             |   3 +-
 nptl/cancellation.c       |  31 +++++++--
 nptl/pthread_cancel.c     |   1 -
 nptl/pthread_exit.c       |   4 +-
 nptl/pthread_testcancel.c |   5 +-
 nptl/tst-cleanup5.c       | 129 ++++++++++++++++++++++++++++++++++++++
 sysdeps/nptl/pthreadP.h   |  14 ++---
 7 files changed, 164 insertions(+), 23 deletions(-)
 create mode 100644 nptl/tst-cleanup5.c
  

Patch

diff --git a/nptl/Makefile b/nptl/Makefile
index b585663974..fa551798d0 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -312,7 +312,8 @@  tests = tst-attr2 tst-attr3 tst-default-attr \
 	tst-pthread-gdb-attach tst-pthread-gdb-attach-static \
 	tst-pthread_exit-nothreads \
 	tst-pthread_exit-nothreads-static \
-	tst-thread-setspecific
+	tst-thread-setspecific \
+	tst-cleanup5
 
 tests-nolibpthread = \
   tst-pthread_exit-nothreads \
diff --git a/nptl/cancellation.c b/nptl/cancellation.c
index f4b08902b2..fef2e9a614 100644
--- a/nptl/cancellation.c
+++ b/nptl/cancellation.c
@@ -43,10 +43,7 @@  __pthread_enable_asynccancel (void)
 						&oldval, newval))
 	{
 	  if (cancel_enabled_and_canceled_and_async (newval))
-	    {
-	      self->result = PTHREAD_CANCELED;
-	      __do_cancel ();
-	    }
+	    __do_cancel ();
 
 	  break;
 	}
@@ -89,3 +86,29 @@  __pthread_disable_asynccancel (int oldtype)
     }
 }
 libc_hidden_def (__pthread_disable_asynccancel)
+
+void
+__exit_thread (void *value)
+{
+  struct pthread *self = THREAD_SELF;
+
+  THREAD_SETMEM (self, result, value);
+
+  int oldval = atomic_load_relaxed (&self->cancelhandling);
+  int newval;
+  do
+    {
+      /* It is required by POSIX XSH 2.9.5 Thread Cancellation under the
+	 heading Thread Cancellation Cleanup Handlers and also avoid further
+	 cancellation wrapper to act on cancellation.  */
+      newval = oldval | CANCELSTATE_BITMASK | EXITING_BITMASK;
+      newval = newval & ~CANCELTYPE_BITMASK;
+      if (oldval == newval)
+	break;
+    }
+  while (!atomic_compare_exchange_weak_acquire (&self->cancelhandling,
+						&oldval, newval));
+
+  __pthread_unwind ((__pthread_unwind_buf_t *)
+		    THREAD_GETMEM (self, cleanup_jmp_buf));
+}
diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c
index e67b2df5cc..c4e571ec21 100644
--- a/nptl/pthread_cancel.c
+++ b/nptl/pthread_cancel.c
@@ -143,7 +143,6 @@  __pthread_cancel (pthread_t th)
 	       pthread_create, so the signal handler may not have been
 	       set up for a self-cancel.  */
 	    {
-	      pd->result = PTHREAD_CANCELED;
 	      if ((newval & CANCELTYPE_BITMASK) != 0)
 		__do_cancel ();
 	    }
diff --git a/nptl/pthread_exit.c b/nptl/pthread_exit.c
index 3e20c03b52..8ff8a4fd25 100644
--- a/nptl/pthread_exit.c
+++ b/nptl/pthread_exit.c
@@ -31,9 +31,7 @@  __pthread_exit (void *value)
                     " must be installed for pthread_exit to work\n");
   }
 
-  THREAD_SETMEM (THREAD_SELF, result, value);
-
-  __do_cancel ();
+  __exit_thread (value);
 }
 libc_hidden_def (__pthread_exit)
 weak_alias (__pthread_exit, pthread_exit)
diff --git a/nptl/pthread_testcancel.c b/nptl/pthread_testcancel.c
index b81928c000..c1fd1e0e3a 100644
--- a/nptl/pthread_testcancel.c
+++ b/nptl/pthread_testcancel.c
@@ -25,10 +25,7 @@  ___pthread_testcancel (void)
   struct pthread *self = THREAD_SELF;
   int cancelhandling = atomic_load_relaxed (&self->cancelhandling);
   if (cancel_enabled_and_canceled (cancelhandling))
-    {
-      self->result = PTHREAD_CANCELED;
-      __do_cancel ();
-    }
+    __do_cancel ();
 }
 versioned_symbol (libc, ___pthread_testcancel, pthread_testcancel, GLIBC_2_34);
 libc_hidden_ver (___pthread_testcancel, __pthread_testcancel)
diff --git a/nptl/tst-cleanup5.c b/nptl/tst-cleanup5.c
new file mode 100644
index 0000000000..726b651c46
--- /dev/null
+++ b/nptl/tst-cleanup5.c
@@ -0,0 +1,129 @@ 
+/* Check if cancellation state and type is correctly set on thread exit.
+   Copyright (C) 2022 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 <support/check.h>
+#include <support/xthread.h>
+#include <support/xunistd.h>
+
+static int pipefds[2];
+static pthread_barrier_t b;
+
+static void
+clh (void *arg)
+{
+  /* Although POSIX state setting either the cancellation state or type is
+     undefined during cleanup handler execution, both calls should be safe
+     since none has any side-effect (they should not change current state
+     neither trigger a pending cancellation).  */
+
+  int state;
+  TEST_VERIFY (pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, &state) == 0);
+  TEST_COMPARE (state, PTHREAD_CANCEL_DISABLE);
+
+  int type;
+  TEST_VERIFY (pthread_setcanceltype (PTHREAD_CANCEL_DEFERRED, &type) == 0);
+  TEST_COMPARE (type, PTHREAD_CANCEL_DEFERRED);
+}
+
+/* Check if a thread with PTHREAD_CANCEL_DEFERRED cancellation on
+   pthread_cleanup_pop sets the correct state and type as pthread_exit.  */
+static void *
+tf_cancel_deferred (void *arg)
+{
+  xpthread_barrier_wait (&b);
+
+  pthread_cleanup_push (clh, NULL);
+
+  char c;
+  read (pipefds[0], &c, 1);
+
+  pthread_cleanup_pop (1);
+
+  return NULL;
+}
+
+/* Check if a thread with PTHREAD_CANCEL_DEFERRED cancellation on
+   blocked read() sets the correct state and type as pthread_exit.  */
+static void *
+tf_testcancel (void *arg)
+{
+  xpthread_barrier_wait (&b);
+
+  pthread_cleanup_push (clh, NULL);
+
+  char c;
+  read (pipefds[0], &c, 1);
+
+  pthread_testcancel ();
+
+  pthread_cleanup_pop (1);
+
+  return NULL;
+}
+
+#define EXIT_EXPECTED_VALUE ((void *) 42)
+
+/* Check if a thread with PTHREAD_CANCEL_DEFERRED cancellation on
+   pthread_exit() sets the correct state and type.  */
+static void *
+tf_exit (void *arg)
+{
+  xpthread_barrier_wait (&b);
+
+  pthread_cleanup_push (clh, NULL);
+
+  pthread_exit (EXIT_EXPECTED_VALUE);
+
+  pthread_cleanup_pop (1);
+
+  return NULL;
+}
+
+static int
+do_test (void)
+{
+  xpipe (pipefds);
+
+  xpthread_barrier_init (&b, NULL, 2);
+  {
+    pthread_t th = xpthread_create (NULL, tf_cancel_deferred, NULL);
+    xpthread_barrier_wait (&b);
+    xpthread_cancel (th);
+    void *r = xpthread_join (th);
+    TEST_VERIFY (r == PTHREAD_CANCELED);
+  }
+
+  {
+    pthread_t th = xpthread_create (NULL, tf_testcancel, NULL);
+    xpthread_barrier_wait (&b);
+    xpthread_cancel (th);
+    void *r = xpthread_join (th);
+    TEST_VERIFY (r == PTHREAD_CANCELED);
+  }
+
+  {
+    pthread_t th = xpthread_create (NULL, tf_exit, NULL);
+    xpthread_barrier_wait (&b);
+    void *r = xpthread_join (th);
+    TEST_VERIFY (r == EXIT_EXPECTED_VALUE);
+  }
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h
index 39af275c25..e6e7430dfe 100644
--- a/sysdeps/nptl/pthreadP.h
+++ b/sysdeps/nptl/pthreadP.h
@@ -269,20 +269,14 @@  extern void __pthread_unregister_cancel (__pthread_unwind_buf_t *__buf)
 libc_hidden_proto (__pthread_unregister_cancel)
 
 /* Called when a thread reacts on a cancellation request.  */
-static inline void
-__attribute ((noreturn, always_inline))
+extern _Noreturn void __exit_thread (void *value) attribute_hidden;
+
+static _Noreturn __always_inline void
 __do_cancel (void)
 {
-  struct pthread *self = THREAD_SELF;
-
-  /* Make sure we get no more cancellations.  */
-  atomic_bit_set (&self->cancelhandling, EXITING_BIT);
-
-  __pthread_unwind ((__pthread_unwind_buf_t *)
-		    THREAD_GETMEM (self, cleanup_jmp_buf));
+  __exit_thread (PTHREAD_CANCELED);
 }
 
-
 /* Internal prototypes.  */
 
 /* Deallocate a thread's stack after optionally making sure the thread