diff options
author | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2016-08-22 09:39:46 -0300 |
---|---|---|
committer | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2016-09-15 11:14:31 -0300 |
commit | 47677f2edc815e85d0383a89b09733e95e5d7302 (patch) | |
tree | e6c5db6e9cff5127be9efe1ffbd4893a3808a245 /nptl/sem_wait.c | |
parent | 49ad334ab10671dabb40eda8beba887ef902f0f3 (diff) | |
download | glibc-47677f2edc815e85d0383a89b09733e95e5d7302.tar glibc-47677f2edc815e85d0383a89b09733e95e5d7302.tar.gz glibc-47677f2edc815e85d0383a89b09733e95e5d7302.tar.bz2 glibc-47677f2edc815e85d0383a89b09733e95e5d7302.zip |
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.
Diffstat (limited to 'nptl/sem_wait.c')
-rw-r--r-- | nptl/sem_wait.c | 13 |
1 files changed, 13 insertions, 0 deletions
diff --git a/nptl/sem_wait.c b/nptl/sem_wait.c index 84b523a768..b242c08d2e 100644 --- a/nptl/sem_wait.c +++ b/nptl/sem_wait.c @@ -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 |