public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
* [glibc/azanella/pthread-multiple-fixes] nptl: Set cancellation type and state on pthread_exit
@ 2021-08-20 17:55 Adhemerval Zanella
  0 siblings, 0 replies; 2+ messages in thread
From: Adhemerval Zanella @ 2021-08-20 17:55 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=7ce598e7e1f725dd324840643b9c71c43457598f

commit 7ce598e7e1f725dd324840643b9c71c43457598f
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Mon May 31 10:47:52 2021 -0300

    nptl: Set cancellation type and state on pthread_exit
    
    It is required by POSIX XSH 2.9.5 Thread Cancellation under the heading
    Thread Cancellation Cleanup Handlers.
    
    Checked x86_64-linux-gnu.

Diff:
---
 nptl/Makefile             |   3 +-
 nptl/cancellation.c       |   7 ++-
 nptl/pthread_cancel.c     |   1 -
 nptl/pthread_exit.c       |   4 +-
 nptl/pthread_testcancel.c |   1 -
 nptl/tst-cleanup5.c       | 157 ++++++++++++++++++++++++++++++++++++++++++++++
 sysdeps/nptl/pthreadP.h   |  18 ++++--
 7 files changed, 180 insertions(+), 11 deletions(-)

diff --git a/nptl/Makefile b/nptl/Makefile
index ff4d590f11..e1e195cc15 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -306,7 +306,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 05962784d5..6478c029de 100644
--- a/nptl/cancellation.c
+++ b/nptl/cancellation.c
@@ -42,7 +42,6 @@ __pthread_enable_asynccancel (void)
       && !(ch & EXITING_BITMASK)
       && !(ch & TERMINATED_BITMASK))
     {
-      THREAD_SETMEM (self, result, PTHREAD_CANCELED);
       __do_cancel ();
     }
 
@@ -64,3 +63,9 @@ __pthread_disable_asynccancel (int oldtype)
   self->canceltype = PTHREAD_CANCEL_DEFERRED;
 }
 libc_hidden_def (__pthread_disable_asynccancel)
+
+void
+__do_cancel (void)
+{
+  __exit_thread (PTHREAD_CANCELED);
+}
diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c
index cc25ff21f3..38f637c5cf 100644
--- a/nptl/pthread_cancel.c
+++ b/nptl/pthread_cancel.c
@@ -107,7 +107,6 @@ __pthread_cancel (pthread_t th)
       __libc_multiple_threads = 1;
 #endif
 
-      THREAD_SETMEM (pd, result, PTHREAD_CANCELED);
       if (pd->cancelstate == PTHREAD_CANCEL_ENABLE
 	  && pd->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS)
 	__do_cancel ();
diff --git a/nptl/pthread_exit.c b/nptl/pthread_exit.c
index 6abf66463e..978f4042ef 100644
--- a/nptl/pthread_exit.c
+++ b/nptl/pthread_exit.c
@@ -32,9 +32,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 31185d89f2..f728a35524 100644
--- a/nptl/pthread_testcancel.c
+++ b/nptl/pthread_testcancel.c
@@ -30,7 +30,6 @@ ___pthread_testcancel (void)
       && !(cancelhandling & EXITING_BITMASK)
       && !(cancelhandling & TERMINATED_BITMASK))
     {
-      THREAD_SETMEM (self, result, PTHREAD_CANCELED);
       __do_cancel ();
     }
 }
diff --git a/nptl/tst-cleanup5.c b/nptl/tst-cleanup5.c
new file mode 100644
index 0000000000..6cc1c141e9
--- /dev/null
+++ b/nptl/tst-cleanup5.c
@@ -0,0 +1,157 @@
+/* Check if cancellation state and type is correctly set on thread exit.
+   Copyright (C) 2021 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_ASYNCHRONOUS cancellation on
+   blocked read() sets the correct state and type as pthread_exit.  */
+static void *
+tf_cancel_async (void *arg)
+{
+  TEST_VERIFY (pthread_setcanceltype (PTHREAD_CANCEL_ASYNCHRONOUS, NULL)
+	       == 0);
+
+  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_cancel_async, 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 374657a2fd..d968a91cfd 100644
--- a/sysdeps/nptl/pthreadP.h
+++ b/sysdeps/nptl/pthreadP.h
@@ -268,20 +268,30 @@ extern void __pthread_unregister_cancel (__pthread_unwind_buf_t *__buf)
      __cleanup_fct_attribute;
 libc_hidden_proto (__pthread_unregister_cancel)
 
-/* Called when a thread reacts on a cancellation request.  */
-static inline void
-__attribute ((noreturn, always_inline))
-__do_cancel (void)
+static _Noreturn inline void
+__exit_thread (void *value)
 {
   struct pthread *self = THREAD_SELF;
 
   /* Make sure we get no more cancellations.  */
   THREAD_ATOMIC_BIT_SET (self, cancelhandling, EXITING_BIT);
 
+  THREAD_SETMEM (self, result, value);
+
+  /* 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.  */
+  THREAD_SETMEM (self, cancelstate, PTHREAD_CANCEL_DISABLE);
+  THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_DEFERRED);
+
   __pthread_unwind ((__pthread_unwind_buf_t *)
 		    THREAD_GETMEM (self, cleanup_jmp_buf));
 }
 
+/* It is a wrapper over __exit_thread (PTHREAD_CANCELED).  It is has its own
+   implementation because it might be called by arch-specific asm code.  */
+_Noreturn void __do_cancel (void) attribute_hidden;
+
 
 /* Internal prototypes.  */


^ permalink raw reply	[flat|nested] 2+ messages in thread

* [glibc/azanella/pthread-multiple-fixes] nptl: Set cancellation type and state on pthread_exit
@ 2021-06-08 20:48 Adhemerval Zanella
  0 siblings, 0 replies; 2+ messages in thread
From: Adhemerval Zanella @ 2021-06-08 20:48 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=12c7358b250a3adf48c1d50b092aa055a1cd8230

commit 12c7358b250a3adf48c1d50b092aa055a1cd8230
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Mon May 31 10:47:52 2021 -0300

    nptl: Set cancellation type and state on pthread_exit
    
    It is required by POSIX XSH 2.9.5 Thread Cancellation under the heading
    Thread Cancellation Cleanup Handlers.
    
    Checked x86_64-linux-gnu.

Diff:
---
 nptl/Makefile             |   3 +-
 nptl/cancellation.c       |   7 ++-
 nptl/pthreadP.h           |  18 ++++--
 nptl/pthread_cancel.c     |   1 -
 nptl/pthread_exit.c       |   4 +-
 nptl/pthread_testcancel.c |   1 -
 nptl/tst-cleanup5.c       | 157 ++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 180 insertions(+), 11 deletions(-)

diff --git a/nptl/Makefile b/nptl/Makefile
index 3e6cf0c21b..3b260277dc 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -307,7 +307,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 05962784d5..6478c029de 100644
--- a/nptl/cancellation.c
+++ b/nptl/cancellation.c
@@ -42,7 +42,6 @@ __pthread_enable_asynccancel (void)
       && !(ch & EXITING_BITMASK)
       && !(ch & TERMINATED_BITMASK))
     {
-      THREAD_SETMEM (self, result, PTHREAD_CANCELED);
       __do_cancel ();
     }
 
@@ -64,3 +63,9 @@ __pthread_disable_asynccancel (int oldtype)
   self->canceltype = PTHREAD_CANCEL_DEFERRED;
 }
 libc_hidden_def (__pthread_disable_asynccancel)
+
+void
+__do_cancel (void)
+{
+  __exit_thread (PTHREAD_CANCELED);
+}
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 675d1de753..ef0b367120 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -268,20 +268,30 @@ extern void __pthread_unregister_cancel (__pthread_unwind_buf_t *__buf)
      __cleanup_fct_attribute;
 libc_hidden_proto (__pthread_unregister_cancel)
 
-/* Called when a thread reacts on a cancellation request.  */
-static inline void
-__attribute ((noreturn, always_inline))
-__do_cancel (void)
+static _Noreturn inline void
+__exit_thread (void *value)
 {
   struct pthread *self = THREAD_SELF;
 
   /* Make sure we get no more cancellations.  */
   THREAD_ATOMIC_BIT_SET (self, cancelhandling, EXITING_BIT);
 
+  THREAD_SETMEM (self, result, value);
+
+  /* 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.  */
+  THREAD_SETMEM (self, cancelstate, PTHREAD_CANCEL_DISABLE);
+  THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_DEFERRED);
+
   __pthread_unwind ((__pthread_unwind_buf_t *)
 		    THREAD_GETMEM (self, cleanup_jmp_buf));
 }
 
+/* It is a wrapper over __exit_thread (PTHREAD_CANCELED).  It is has its own
+   implementation because it might be called by arch-specific asm code.  */
+_Noreturn void __do_cancel (void) attribute_hidden;
+
 
 /* Internal prototypes.  */
 
diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c
index 0698cd2046..577ff6c746 100644
--- a/nptl/pthread_cancel.c
+++ b/nptl/pthread_cancel.c
@@ -103,7 +103,6 @@ __pthread_cancel (pthread_t th)
       __libc_multiple_threads = 1;
 #endif
 
-      THREAD_SETMEM (pd, result, PTHREAD_CANCELED);
       if (pd->cancelstate == PTHREAD_CANCEL_ENABLE
 	  && pd->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS)
 	__do_cancel ();
diff --git a/nptl/pthread_exit.c b/nptl/pthread_exit.c
index 6abf66463e..978f4042ef 100644
--- a/nptl/pthread_exit.c
+++ b/nptl/pthread_exit.c
@@ -32,9 +32,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 920374643a..e6adf12580 100644
--- a/nptl/pthread_testcancel.c
+++ b/nptl/pthread_testcancel.c
@@ -30,7 +30,6 @@ ___pthread_testcancel (void)
       && !(cancelhandling & EXITING_BITMASK)
       && !(cancelhandling & TERMINATED_BITMASK))
     {
-      THREAD_SETMEM (self, result, PTHREAD_CANCELED);
       __do_cancel ();
     }
 }
diff --git a/nptl/tst-cleanup5.c b/nptl/tst-cleanup5.c
new file mode 100644
index 0000000000..6cc1c141e9
--- /dev/null
+++ b/nptl/tst-cleanup5.c
@@ -0,0 +1,157 @@
+/* Check if cancellation state and type is correctly set on thread exit.
+   Copyright (C) 2021 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_ASYNCHRONOUS cancellation on
+   blocked read() sets the correct state and type as pthread_exit.  */
+static void *
+tf_cancel_async (void *arg)
+{
+  TEST_VERIFY (pthread_setcanceltype (PTHREAD_CANCEL_ASYNCHRONOUS, NULL)
+	       == 0);
+
+  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_cancel_async, 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>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-08-20 17:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 17:55 [glibc/azanella/pthread-multiple-fixes] nptl: Set cancellation type and state on pthread_exit Adhemerval Zanella
  -- strict thread matches above, loose matches on Subject: below --
2021-06-08 20:48 Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).