nptl: Fix sem_wait and sem_timedwait cancellation (BZ#18243)
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 testcase also does 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.
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 (cleanup): Remove wrong cancellation point.
(tf): Fix check for uncontended case.
(do_test): Likewise.
* nptl/tst-cancel13.c (cleanup): Remove wrong cancellation point.
(tf): Fix check for uncontended case.
(do_test): Likewise.
* nptl/tst-cancel14.c (cleanup): Remove wrong cancellation point.
(tf): Fix check for uncontended case.
(do_test): Likewise.
* nptl/tst-cancel15.c (cleanup): Remove wrong cancellation point.
(tf): Fix check for uncontended case.
(do_test): Likewise.
This commit is contained in:
Adhemerval Zanella
parent
49ad334ab1
commit
47677f2edc
20
ChangeLog
20
ChangeLog
|
|
@ -1,5 +1,25 @@
|
|||
2016-09-15 Adhemerval Zanella <adhemerval.zanella@linaro.org>
|
||||
|
||||
[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 (cleanup): Remove wrong cancellation point.
|
||||
(tf): Fix check for uncontended case.
|
||||
(do_test): Likewise.
|
||||
* nptl/tst-cancel13.c (cleanup): Remove wrong cancellation point.
|
||||
(tf): Fix check for uncontended case.
|
||||
(do_test): Likewise.
|
||||
* nptl/tst-cancel14.c (cleanup): Remove wrong cancellation point.
|
||||
(tf): Fix check for uncontended case.
|
||||
(do_test): Likewise.
|
||||
* nptl/tst-cancel15.c (cleanup): Remove wrong cancellation point.
|
||||
(tf): Fix check for uncontended case.
|
||||
(do_test): Likewise.
|
||||
|
||||
* sysdeps/sparc/sparc32/sem_wait.c: Remove file.
|
||||
* sysdeps/sparc/sparc32/sparcv9/sem_wait.c: Likewise.
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -30,6 +30,9 @@ sem_timedwait (sem_t *sem, const struct timespec *abstime)
|
|||
return -1;
|
||||
}
|
||||
|
||||
/* Check sem_wait.c for a more detailed explanation why it is required. */
|
||||
__pthread_testcancel ();
|
||||
|
||||
if (__new_sem_wait_fast ((struct new_sem *) sem, 0) == 0)
|
||||
return 0;
|
||||
else
|
||||
|
|
|
|||
|
|
@ -23,6 +23,19 @@
|
|||
int
|
||||
__new_sem_wait (sem_t *sem)
|
||||
{
|
||||
/* We need to check whether we need to act upon a cancellation request here
|
||||
because POSIX specifies that cancellation points "shall occur" in
|
||||
sem_wait and sem_timedwait, which also means that they need to check
|
||||
this regardless whether they block or not (unlike "may occur"
|
||||
functions). See the POSIX Rationale for this requirement: Section
|
||||
"Thread Cancellation Overview" [1] and austin group issue #1076 [2]
|
||||
for thoughs on why this may be a suboptimal design.
|
||||
|
||||
[1] http://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap02.html
|
||||
[2] http://austingroupbugs.net/view.php?id=1076 for thoughts on why this
|
||||
*/
|
||||
__pthread_testcancel ();
|
||||
|
||||
if (__new_sem_wait_fast ((struct new_sem *) sem, 0) == 0)
|
||||
return 0;
|
||||
else
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
@ -39,8 +40,6 @@ cleanup (void *arg)
|
|||
puts ("second call to cleanup");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
printf ("cleanup call #%d\n", ncall);
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -52,17 +51,15 @@ tf (void *arg)
|
|||
int e = pthread_barrier_wait (&bar);
|
||||
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
|
||||
{
|
||||
puts ("tf: 1st barrier_wait failed");
|
||||
puts ("error: tf: 1st barrier_wait failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
/* This call should block and be cancelable. */
|
||||
/* This call should be cancelable. */
|
||||
sem_wait (&sem);
|
||||
|
||||
pthread_cleanup_pop (0);
|
||||
|
||||
puts ("sem_wait returned");
|
||||
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
|
@ -74,47 +71,47 @@ do_test (void)
|
|||
|
||||
if (pthread_barrier_init (&bar, NULL, 2) != 0)
|
||||
{
|
||||
puts ("barrier_init failed");
|
||||
puts ("error: barrier_init failed");
|
||||
exit (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");
|
||||
puts ("error: sem_init failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
if (pthread_create (&th, NULL, tf, NULL) != 0)
|
||||
{
|
||||
puts ("create failed");
|
||||
puts ("error: create failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
/* Check whether cancellation is honored even before sem_wait does
|
||||
anything. */
|
||||
if (pthread_cancel (th) != 0)
|
||||
{
|
||||
puts ("1st cancel failed");
|
||||
puts ("error: pthread_cancel failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
int e = pthread_barrier_wait (&bar);
|
||||
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
|
||||
{
|
||||
puts ("1st barrier_wait failed");
|
||||
puts ("error: 1st barrier_wait failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
void *r;
|
||||
if (pthread_join (th, &r) != 0)
|
||||
{
|
||||
puts ("join failed");
|
||||
puts ("error: pthread_join failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
if (r != PTHREAD_CANCELED)
|
||||
{
|
||||
puts ("thread not canceled");
|
||||
puts ("error: thread not canceled");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
@ -39,8 +40,6 @@ cleanup (void *arg)
|
|||
puts ("second call to cleanup");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
printf ("cleanup call #%d\n", ncall);
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -52,7 +51,7 @@ tf (void *arg)
|
|||
int e = pthread_barrier_wait (&bar);
|
||||
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
|
||||
{
|
||||
puts ("tf: 1st barrier_wait failed");
|
||||
puts ("error: tf: 1st barrier_wait failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
|
|
@ -61,8 +60,6 @@ tf (void *arg)
|
|||
|
||||
pthread_cleanup_pop (0);
|
||||
|
||||
puts ("sem_wait returned");
|
||||
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
|
@ -74,26 +71,28 @@ do_test (void)
|
|||
|
||||
if (pthread_barrier_init (&bar, NULL, 2) != 0)
|
||||
{
|
||||
puts ("barrier_init failed");
|
||||
puts ("error: barrier_init failed");
|
||||
exit (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");
|
||||
puts ("error: sem_init failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
if (pthread_create (&th, NULL, tf, NULL) != 0)
|
||||
{
|
||||
puts ("create failed");
|
||||
puts ("error: create failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
int e = pthread_barrier_wait (&bar);
|
||||
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
|
||||
{
|
||||
puts ("1st barrier_wait failed");
|
||||
puts ("error: 1st barrier_wait failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
|
|
@ -103,14 +102,14 @@ do_test (void)
|
|||
/* Check whether cancellation is honored when waiting in sem_wait. */
|
||||
if (pthread_cancel (th) != 0)
|
||||
{
|
||||
puts ("1st cancel failed");
|
||||
puts ("error: 1st cancel failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
void *r;
|
||||
if (pthread_join (th, &r) != 0)
|
||||
{
|
||||
puts ("join failed");
|
||||
puts ("error: join failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
@ -40,8 +41,6 @@ cleanup (void *arg)
|
|||
puts ("second call to cleanup");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
printf ("cleanup call #%d\n", ncall);
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -53,7 +52,7 @@ tf (void *arg)
|
|||
int e = pthread_barrier_wait (&bar);
|
||||
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
|
||||
{
|
||||
puts ("tf: 1st barrier_wait failed");
|
||||
puts ("error: tf: 1st barrier_wait failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
|
|
@ -71,8 +70,6 @@ tf (void *arg)
|
|||
|
||||
pthread_cleanup_pop (0);
|
||||
|
||||
puts ("sem_timedwait returned");
|
||||
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
|
@ -84,19 +81,19 @@ do_test (void)
|
|||
|
||||
if (pthread_barrier_init (&bar, NULL, 2) != 0)
|
||||
{
|
||||
puts ("barrier_init failed");
|
||||
puts ("error: barrier_init failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
if (sem_init (&sem, 0, 1) != 0)
|
||||
{
|
||||
puts ("sem_init failed");
|
||||
puts ("error: sem_init failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
if (pthread_create (&th, NULL, tf, NULL) != 0)
|
||||
{
|
||||
puts ("create failed");
|
||||
puts ("error: create failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
|
|
@ -104,7 +101,7 @@ do_test (void)
|
|||
anything. */
|
||||
if (pthread_cancel (th) != 0)
|
||||
{
|
||||
puts ("1st cancel failed");
|
||||
puts ("error: 1st cancel failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
@ -40,8 +41,6 @@ cleanup (void *arg)
|
|||
puts ("second call to cleanup");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
printf ("cleanup call #%d\n", ncall);
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -55,7 +54,7 @@ tf (void *arg)
|
|||
e = pthread_barrier_wait (&bar);
|
||||
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
|
||||
{
|
||||
puts ("tf: 1st barrier_wait failed");
|
||||
puts ("error: tf: 1st barrier_wait failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
|
|
@ -74,8 +73,6 @@ tf (void *arg)
|
|||
|
||||
pthread_cleanup_pop (0);
|
||||
|
||||
printf ("sem_timedwait returned, e = %d, errno = %d\n", e, errno);
|
||||
|
||||
return NULL;
|
||||
}
|
||||
|
||||
|
|
@ -87,26 +84,26 @@ do_test (void)
|
|||
|
||||
if (pthread_barrier_init (&bar, NULL, 2) != 0)
|
||||
{
|
||||
puts ("barrier_init failed");
|
||||
puts ("error: barrier_init failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
if (sem_init (&sem, 0, 0) != 0)
|
||||
{
|
||||
puts ("sem_init failed");
|
||||
puts ("error: sem_init failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
if (pthread_create (&th, NULL, tf, NULL) != 0)
|
||||
{
|
||||
puts ("create failed");
|
||||
puts ("error: create failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
int e = pthread_barrier_wait (&bar);
|
||||
if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
|
||||
{
|
||||
puts ("1st barrier_wait failed");
|
||||
puts ("error: 1st barrier_wait failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
|
|
@ -116,20 +113,20 @@ do_test (void)
|
|||
/* Check whether cancellation is honored when waiting in sem_timedwait. */
|
||||
if (pthread_cancel (th) != 0)
|
||||
{
|
||||
puts ("1st cancel failed");
|
||||
puts ("error: 1st cancel failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
void *r;
|
||||
if (pthread_join (th, &r) != 0)
|
||||
{
|
||||
puts ("join failed");
|
||||
puts ("error: join failed");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
if (r != PTHREAD_CANCELED)
|
||||
{
|
||||
puts ("thread not canceled");
|
||||
puts ("error: thread not canceled");
|
||||
exit (1);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue