This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH 4/5] nptl: Fix sem_wait and sem_timedwait cancellation
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Mon, 22 Aug 2016 11:27:32 -0300
- Subject: [PATCH 4/5] nptl: Fix sem_wait and sem_timedwait cancellation
- Authentication-results: sourceware.org; auth=none
- References: <1471876053-780-1-git-send-email-adhemerval.zanella@linaro.org>
This patch fixes both sem_wait and sem_timedwait cancellation point for
uncontended case. In this scenario only atomics are involved and thus
the futex cancellable call is not issue and a pending cancellation signal
is not handled.
The fix is straighforward by calling pthread_testcancel is both function
start. Although it would be simpler to call CANCELLATION_P directly, I
decided to add an internal pthread_testcancel alias and use it to export
less internal implementation on such function. A possible change on
how pthread_testcancel is internally implemented would lead to either
continue to force use CANCELLATION_P or to adjust its every use.
GLIBC testcases do have tests for uncontended cases, test-cancel12
and test-cancel14.c, however both are flawed by adding another
cancellation point just after thread pthread_cleanup_pop:
47 static void *
48 tf (void *arg)
49 {
50 pthread_cleanup_push (cleanup, NULL);
51
52 int e = pthread_barrier_wait (&bar);
53 if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
54 {
55 puts ("tf: 1st barrier_wait failed");
56 exit (1);
57 }
58
59 /* This call should block and be cancelable. */
60 sem_wait (&sem);
61
62 pthread_cleanup_pop (0);
63
64 puts ("sem_wait returned");
65
66 return NULL;
67 }
So sem_{timed}wait does not act on cancellation, pthread_cleanup_pop executes
'cleanup' and then 'puts' acts on cancellation. Since pthread_cleanup_pop
removed the clean-up handler, it will ran only once and thus it won't accuse
an error to indicate sem_wait has not acted on the cancellation signal.
This patch also fixes this behavior by removing the cancellation point 'puts'.
It also adds some cleanup on all sem_{timed}wait cancel tests, mostly to avoid
calling 'exit' and some error formatting.
It partially fixes BZ #18243. Checked on x86_64.
[BZ #18243]
* nptl/pthreadP.h (__pthread_testcancel): Add prototype and hidden_proto.
* nptl/pthread_testcancel.c (pthread_cancel): Add internal aliais
definition.
* nptl/sem_timedwait.c (sem_timedwait): Add cancellation check for
uncontended case.
* nptl/sem_wait.c (__new_sem_wait): Likewise.
* nptl/tst-cancel12.c (ncall): New global variable.
(thread_ret): Likewise.
(clean): Remove cancellation points.
(tf): Fix check for uncontended case and remove exit calls.
(do_test): Likewise.
* nptl/tst-cancel13.c (ncall): New global variable.
(thread_ret): Likewise.
(clean): Remove cancellation points.
(tf): Fix check for uncontended case and remove exit calls.
(do_test): Likewise.
* nptl/tst-cancel14.c (ncall): New global variable.
(thread_ret): Likewise.
(clean): Remove cancellation points.
(tf): Fix check for uncontended case and remove exit calls.
(do_test): Likewise.
* nptl/tst-cancel15.c (ncall): New global variable.
(thread_ret): Likewise.
(clean): Remove cancellation points.
(tf): Fix check for uncontended case and remove exit calls.
(do_test): Likewise.
---
nptl/pthreadP.h | 2 ++
nptl/pthread_testcancel.c | 4 ++-
nptl/sem_timedwait.c | 3 +++
nptl/sem_wait.c | 3 +++
nptl/tst-cancel12.c | 64 +++++++++++++++++++++++------------------------
nptl/tst-cancel13.c | 49 ++++++++++++++++++++----------------
nptl/tst-cancel14.c | 43 ++++++++++++++++---------------
nptl/tst-cancel15.c | 53 +++++++++++++++++++++------------------
9 files changed, 148 insertions(+), 101 deletions(-)
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 4edc74b..6e0dd09 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -491,6 +491,7 @@ extern int __pthread_setcanceltype (int type, int *oldtype);
extern int __pthread_enable_asynccancel (void) attribute_hidden;
extern void __pthread_disable_asynccancel (int oldtype)
internal_function attribute_hidden;
+extern void __pthread_testcancel (void);
#if IS_IN (libpthread)
hidden_proto (__pthread_mutex_init)
@@ -505,6 +506,7 @@ hidden_proto (__pthread_getspecific)
hidden_proto (__pthread_setspecific)
hidden_proto (__pthread_once)
hidden_proto (__pthread_setcancelstate)
+hidden_proto (__pthread_testcancel)
#endif
extern int __pthread_cond_broadcast_2_0 (pthread_cond_2_0_t *cond);
diff --git a/nptl/pthread_testcancel.c b/nptl/pthread_testcancel.c
index 51be09f..fdef699 100644
--- a/nptl/pthread_testcancel.c
+++ b/nptl/pthread_testcancel.c
@@ -21,7 +21,9 @@
void
-pthread_testcancel (void)
+__pthread_testcancel (void)
{
CANCELLATION_P (THREAD_SELF);
}
+strong_alias (__pthread_testcancel, pthread_testcancel)
+hidden_def (__pthread_testcancel)
diff --git a/nptl/sem_timedwait.c b/nptl/sem_timedwait.c
index 1aab25a..8253021 100644
--- a/nptl/sem_timedwait.c
+++ b/nptl/sem_timedwait.c
@@ -30,6 +30,9 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
return -1;
}
+ /* Check cancellation for uncontended case. */
+ __pthread_testcancel ();
+
if (__new_sem_wait_fast ((struct new_sem *) sem, 0) == 0)
return 0;
else
diff --git a/nptl/sem_wait.c b/nptl/sem_wait.c
index 84b523a..4928c7d 100644
--- a/nptl/sem_wait.c
+++ b/nptl/sem_wait.c
@@ -23,6 +23,9 @@
int
__new_sem_wait (sem_t *sem)
{
+ /* Check cancellation for uncontended case. */
+ __pthread_testcancel ();
+
if (__new_sem_wait_fast ((struct new_sem *) sem, 0) == 0)
return 0;
else
diff --git a/nptl/tst-cancel12.c b/nptl/tst-cancel12.c
index ac8f5a0..9d93ddb 100644
--- a/nptl/tst-cancel12.c
+++ b/nptl/tst-cancel12.c
@@ -1,4 +1,5 @@
-/* Copyright (C) 2003-2016 Free Software Foundation, Inc.
+/* Test sem_wait cancellation for uncontended case.
+ Copyright (C) 2003-2016 Free Software Foundation, Inc.
This file is part of the GNU C Library.
Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
@@ -23,104 +24,101 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
-
+#include <stdbool.h>
static pthread_barrier_t bar;
static sem_t sem;
-
+static int ncall;
+static volatile int thread_ret;
static void
cleanup (void *arg)
{
- static int ncall;
-
if (++ncall != 1)
{
puts ("second call to cleanup");
- exit (1);
+ thread_ret = 1;
}
-
- printf ("cleanup call #%d\n", ncall);
}
-
static void *
tf (void *arg)
{
+ thread_ret = 0;
+ ncall = 0;
+
pthread_cleanup_push (cleanup, NULL);
int e = pthread_barrier_wait (&bar);
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
{
- puts ("tf: 1st barrier_wait failed");
- exit (1);
+ puts ("error: tf: 1st barrier_wait failed");
+ thread_ret = 1;
+ return NULL;
}
- /* This call should block and be cancelable. */
sem_wait (&sem);
pthread_cleanup_pop (0);
- puts ("sem_wait returned");
-
return NULL;
}
-
static int
do_test (void)
{
pthread_t th;
+ thread_ret = 0;
+
if (pthread_barrier_init (&bar, NULL, 2) != 0)
{
- puts ("barrier_init failed");
- exit (1);
+ puts ("error: barrier_init failed");
+ return 1;
}
+ /* A value higher than 0 will check for uncontended pthread cancellation,
+ where the sem_wait operation will return immediatelly. */
if (sem_init (&sem, 0, 1) != 0)
{
- puts ("sem_init failed");
- exit (1);
+ puts ("error: sem_init failed");
+ return 1;
}
if (pthread_create (&th, NULL, tf, NULL) != 0)
{
- puts ("create failed");
- exit (1);
+ puts ("error: create failed");
+ return 1;
}
- /* Check whether cancellation is honored even before sem_wait does
- anything. */
if (pthread_cancel (th) != 0)
{
- puts ("1st cancel failed");
- exit (1);
+ puts ("error: pthread_cancel failed");
+ return 1;
}
int e = pthread_barrier_wait (&bar);
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
{
- puts ("1st barrier_wait failed");
- exit (1);
+ puts ("error: 1st barrier_wait failed");
+ return 1;
}
void *r;
if (pthread_join (th, &r) != 0)
{
- puts ("join failed");
- exit (1);
+ puts ("error: pthread_join failed");
+ return 1;
}
if (r != PTHREAD_CANCELED)
{
- puts ("thread not canceled");
- exit (1);
+ puts ("error: thread not canceled");
+ return 1;
}
- return 0;
+ return thread_ret;
}
-
#define TEST_FUNCTION do_test ()
#include "../test-skeleton.c"
diff --git a/nptl/tst-cancel13.c b/nptl/tst-cancel13.c
index c84780d..7c7fd6b 100644
--- a/nptl/tst-cancel13.c
+++ b/nptl/tst-cancel13.c
@@ -1,4 +1,5 @@
-/* Copyright (C) 2003-2016 Free Software Foundation, Inc.
+/* Test sem_wait cancellation for contended case.
+ Copyright (C) 2003-2016 Free Software Foundation, Inc.
This file is part of the GNU C Library.
Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
@@ -27,6 +28,8 @@
static pthread_barrier_t bar;
static sem_t sem;
+static int ncall;
+static volatile int thread_ret;
static void
@@ -37,23 +40,24 @@ cleanup (void *arg)
if (++ncall != 1)
{
puts ("second call to cleanup");
- exit (1);
+ thread_ret = 1;
}
-
- printf ("cleanup call #%d\n", ncall);
}
-
static void *
tf (void *arg)
{
+ thread_ret = 0;
+ ncall = 0;
+
pthread_cleanup_push (cleanup, NULL);
int e = pthread_barrier_wait (&bar);
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
{
- puts ("tf: 1st barrier_wait failed");
- exit (1);
+ puts ("error: tf: 1st barrier_wait failed");
+ thread_ret = 1;
+ return NULL;
}
/* This call should block and be cancelable. */
@@ -61,8 +65,6 @@ tf (void *arg)
pthread_cleanup_pop (0);
- puts ("sem_wait returned");
-
return NULL;
}
@@ -71,30 +73,33 @@ static int
do_test (void)
{
pthread_t th;
+ thread_ret = 0;
if (pthread_barrier_init (&bar, NULL, 2) != 0)
{
- puts ("barrier_init failed");
- exit (1);
+ puts ("error: barrier_init failed");
+ return 1;
}
+ /* A value equal to 0 will check for contended pthread cancellation,
+ where the sem_wait operation will block. */
if (sem_init (&sem, 0, 0) != 0)
{
- puts ("sem_init failed");
- exit (1);
+ puts ("error: sem_init failed");
+ return 1;
}
if (pthread_create (&th, NULL, tf, NULL) != 0)
{
- puts ("create failed");
- exit (1);
+ puts ("error: create failed");
+ return 1;
}
int e = pthread_barrier_wait (&bar);
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
{
- puts ("1st barrier_wait failed");
- exit (1);
+ puts ("error: 1st barrier_wait failed");
+ return 1;
}
/* Give the child a chance to go to sleep in sem_wait. */
@@ -103,15 +108,15 @@ do_test (void)
/* Check whether cancellation is honored when waiting in sem_wait. */
if (pthread_cancel (th) != 0)
{
- puts ("1st cancel failed");
- exit (1);
+ puts ("error: 1st cancel failed");
+ return 1;
}
void *r;
if (pthread_join (th, &r) != 0)
{
- puts ("join failed");
- exit (1);
+ puts ("error: join failed");
+ return 1;
}
if (r != PTHREAD_CANCELED)
@@ -120,7 +125,7 @@ do_test (void)
exit (1);
}
- return 0;
+ return thread_ret;
}
diff --git a/nptl/tst-cancel14.c b/nptl/tst-cancel14.c
index 3810d2b..dbd0394 100644
--- a/nptl/tst-cancel14.c
+++ b/nptl/tst-cancel14.c
@@ -1,4 +1,5 @@
-/* Copyright (C) 2003-2016 Free Software Foundation, Inc.
+/* Test sem_timedwait cancellation for uncontended case.
+ Copyright (C) 2003-2016 Free Software Foundation, Inc.
This file is part of the GNU C Library.
Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
@@ -28,33 +29,35 @@
static pthread_barrier_t bar;
static sem_t sem;
+static int ncall;
+static volatile int thread_ret;
static void
cleanup (void *arg)
{
- static int ncall;
-
if (++ncall != 1)
{
puts ("second call to cleanup");
- exit (1);
+ thread_ret = 1;
}
-
- printf ("cleanup call #%d\n", ncall);
}
static void *
tf (void *arg)
{
+ thread_ret = 0;
+ ncall = 0;
+
pthread_cleanup_push (cleanup, NULL);
int e = pthread_barrier_wait (&bar);
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
{
puts ("tf: 1st barrier_wait failed");
- exit (1);
+ thread_ret = 1;
+ return NULL;
}
struct timeval tv;
@@ -71,8 +74,6 @@ tf (void *arg)
pthread_cleanup_pop (0);
- puts ("sem_timedwait returned");
-
return NULL;
}
@@ -82,44 +83,46 @@ do_test (void)
{
pthread_t th;
+ thread_ret = 0;
+
if (pthread_barrier_init (&bar, NULL, 2) != 0)
{
- puts ("barrier_init failed");
- exit (1);
+ puts ("error: barrier_init failed");
+ return 1;
}
if (sem_init (&sem, 0, 1) != 0)
{
- puts ("sem_init failed");
- exit (1);
+ puts ("error: sem_init failed");
+ return 1;
}
if (pthread_create (&th, NULL, tf, NULL) != 0)
{
- puts ("create failed");
- exit (1);
+ puts ("error: create failed");
+ return 1;
}
/* Check whether cancellation is honored even before sem_timedwait does
anything. */
if (pthread_cancel (th) != 0)
{
- puts ("1st cancel failed");
- exit (1);
+ puts ("error: 1st cancel failed");
+ return 1;
}
int e = pthread_barrier_wait (&bar);
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
{
puts ("1st barrier_wait failed");
- exit (1);
+ return 1;
}
void *r;
if (pthread_join (th, &r) != 0)
{
puts ("join failed");
- exit (1);
+ return 1;
}
if (r != PTHREAD_CANCELED)
@@ -128,7 +131,7 @@ do_test (void)
exit (1);
}
- return 0;
+ return thread_ret;
}
diff --git a/nptl/tst-cancel15.c b/nptl/tst-cancel15.c
index f413839..c86164e 100644
--- a/nptl/tst-cancel15.c
+++ b/nptl/tst-cancel15.c
@@ -1,4 +1,5 @@
-/* Copyright (C) 2003-2016 Free Software Foundation, Inc.
+/* Test sem_timedwait cancellation for contended case.
+ Copyright (C) 2003-2016 Free Software Foundation, Inc.
This file is part of the GNU C Library.
Contributed by Ulrich Drepper <drepper@redhat.com>, 2003.
@@ -28,26 +29,27 @@
static pthread_barrier_t bar;
static sem_t sem;
+static int ncall;
+static volatile int thread_ret;
static void
cleanup (void *arg)
{
- static int ncall;
-
if (++ncall != 1)
{
puts ("second call to cleanup");
- exit (1);
+ thread_ret = 1;
}
-
- printf ("cleanup call #%d\n", ncall);
}
static void *
tf (void *arg)
{
+ thread_ret = 0;
+ ncall = 0;
+
int e;
pthread_cleanup_push (cleanup, NULL);
@@ -55,8 +57,9 @@ tf (void *arg)
e = pthread_barrier_wait (&bar);
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
{
- puts ("tf: 1st barrier_wait failed");
- exit (1);
+ puts ("error: tf: 1st barrier_wait failed");
+ thread_ret = 1;
+ return NULL;
}
struct timeval tv;
@@ -74,8 +77,6 @@ tf (void *arg)
pthread_cleanup_pop (0);
- printf ("sem_timedwait returned, e = %d, errno = %d\n", e, errno);
-
return NULL;
}
@@ -85,29 +86,31 @@ do_test (void)
{
pthread_t th;
+ thread_ret = 0;
+
if (pthread_barrier_init (&bar, NULL, 2) != 0)
{
- puts ("barrier_init failed");
- exit (1);
+ puts ("error: barrier_init failed");
+ return 1;
}
if (sem_init (&sem, 0, 0) != 0)
{
- puts ("sem_init failed");
- exit (1);
+ puts ("error: sem_init failed");
+ return 1;
}
if (pthread_create (&th, NULL, tf, NULL) != 0)
{
- puts ("create failed");
- exit (1);
+ puts ("error: create failed");
+ return 1;
}
int e = pthread_barrier_wait (&bar);
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
{
- puts ("1st barrier_wait failed");
- exit (1);
+ puts ("error: 1st barrier_wait failed");
+ return 1;
}
/* Give the child a chance to go to sleep in sem_wait. */
@@ -116,24 +119,24 @@ do_test (void)
/* Check whether cancellation is honored when waiting in sem_timedwait. */
if (pthread_cancel (th) != 0)
{
- puts ("1st cancel failed");
- exit (1);
+ puts ("error: 1st cancel failed");
+ return 1;
}
void *r;
if (pthread_join (th, &r) != 0)
{
- puts ("join failed");
- exit (1);
+ puts ("error: join failed");
+ return 1;
}
if (r != PTHREAD_CANCELED)
{
- puts ("thread not canceled");
- exit (1);
+ puts ("error: thread not canceled");
+ return 1;
}
- return 0;
+ return thread_ret;
}
--
2.7.4