From patchwork Mon Aug 22 14:27:32 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 661491 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3sHwt22Gjxz9sDf for ; Tue, 23 Aug 2016 00:28:42 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=Gp3iLAXj; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:in-reply-to :references; q=dns; s=default; b=KcKI01dRrDAEKZL1efhLeRdSqFJtIjr jkz8K9lZzWpVtqoCH/NXyVkvPo6Qm/JB1VPKTecAlBhhHx/tPw0B4s1LvmSsLyPX xVxgPh5wIGdq8PYJ7aTWy3uCMVcWoNWsDr6Ka+Ti4DA0Bl+RLU8PhTvGlRY7nu9E ya2hoi33n5IQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:in-reply-to :references; s=default; bh=0fqfW+kvUv+8nT5HPjKZ77E8NTQ=; b=Gp3iL AXjZ6nUk2Cch4ojXhw8xGRx7IEMZwbno/0hLRqZx2X5xcR6hDCp3rAh40zvmovcl mbjcFnUJscQGp2we2X81I1DSWLRvXssAp5fLjBPD4Mil5uIc5otkzFNg90JY/H2z quOuxBckSpqUHo08zV8mn8ab/2m5eQugzu7sLQ= Received: (qmail 72536 invoked by alias); 22 Aug 2016 14:27:53 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 72354 invoked by uid 89); 22 Aug 2016 14:27:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=sem, 8631, contributed, acted X-HELO: mail-ua0-f171.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=wEVL08t0q95C8jeEbUy7Sp9iP3dBn1iqAcpTL9yVDEM=; b=Q11DuG7sXzh61Zg1DU7iGE6DfQe4UaJupTO8larsRs/mFyUZe+PI2CwnEdI59WxWqV rWuG8TyJsxD3ZjYSbRpf/yLyGrvCeH0i4JHLMTpuCVG98vE5HAe0CBep8kfiTCUK9vPi RUpJSyllxSiamcXsSZZlzi1oyE/3tNZVHTps9wnSJPmecThyoB5T1fkdz/I40M+DsjFU /QVo/IBNo6PKjjNG1LMIdBW387LC/FJfbAKaKc7f1BJFpFrvXZSFATs5AE6VAuWyF7No iQbrOCGT8mAvrMxK3nP3YNUF0hHztwxj9O1y9HKEquiBBTjwM7kEDjI0dKLXw/QEiWPU vHzA== X-Gm-Message-State: AEkoousSbtl/uoIjbRifR0CcWzt948NcrrKlvTcyjaI/Ce1S58v5P8g0P4KYvxw9HlWrmBLr X-Received: by 10.31.203.196 with SMTP id b187mr11084778vkg.4.1471876063255; Mon, 22 Aug 2016 07:27:43 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH 4/5] nptl: Fix sem_wait and sem_timedwait cancellation Date: Mon, 22 Aug 2016 11:27:32 -0300 Message-Id: <1471876053-780-4-git-send-email-adhemerval.zanella@linaro.org> In-Reply-To: <1471876053-780-1-git-send-email-adhemerval.zanella@linaro.org> 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 , 2003. @@ -23,104 +24,101 @@ #include #include #include - +#include 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 , 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 , 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 , 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; }