From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92124 invoked by alias); 21 Oct 2019 13:29:13 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 92112 invoked by uid 89); 21 Oct 2019 13:29:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-9.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=cancellation, timer, HContent-Transfer-Encoding:8bit X-HELO: mail-qk1-f194.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:openpgp:autocrypt:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=PCV7WVqXGfZHuu+Oldq0zo8KREJCg2U4QVLhfpsbu38=; b=uIX8m9ondVN+jRL/YsqSyYxpSOdedJUMMfTpIxwcb00w9VRT+SVFPr/afWss4/g7a6 1mJUHknyqBlT4wwq3boVJVr+PewSNPuYPEkVgD/2PxlivWkmbo2QOV02el3uV+CGJWNC tza4tBxW4mDw0fdWllOKaKLeaszDHEeb13RrxgFtyDAMtpJirnmmJ8kbr+DaEjJQTstn tFIpGrpsuypj4iXrHxDzD8QN+MCRZv6IigqlGOaPodVuSmaIFYXdgdLR3tYT+vfUuT98 DIcOVg5Zb7LWkgfgcZCyjgmqfWihfd9O6Xy9Ztal2JAiepkm70E66CuLSsf0hJvcgU8g IjOA== Return-Path: Subject: Re: Internal SIGTIMER use (was: Re: [PATCH v3 02/21] nptl: Fix Race conditions in pthread cancellation (BZ#12683)) To: Florian Weimer Cc: libc-alpha@sourceware.org References: <20191014205656.29834-1-adhemerval.zanella@linaro.org> <20191014205656.29834-3-adhemerval.zanella@linaro.org> <87y2xirzyx.fsf_-_@oldenburg2.str.redhat.com> From: Adhemerval Zanella Openpgp: preference=signencrypt Message-ID: <765b6ec6-d301-eee1-55b9-28d2826e51f5@linaro.org> Date: Mon, 21 Oct 2019 13:29:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <87y2xirzyx.fsf_-_@oldenburg2.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2019-10/txt/msg00616.txt.bz2 On 18/10/2019 09:37, Florian Weimer wrote: > * Adhemerval Zanella: > >> - sa.sa_sigaction = sigcancel_handler; >> - sa.sa_flags = SA_SIGINFO; >> - (void) __libc_sigaction (SIGCANCEL, &sa, NULL); >> + { >> + struct sigaction sa; >> + sa.sa_sigaction = sigcancel_handler; >> + /* The signal handle should be non-interruptible to avoid the risk of >> + spurious EINTR caused by SIGCANCEL sent to process or if pthread_cancel >> + is called while cancellation is disabled in the target thread. */ >> + sa.sa_flags = SA_SIGINFO | SA_RESTART; >> + sa.sa_mask = SIGALL_SET; >> + __libc_sigaction (SIGCANCEL, &sa, NULL); >> + } > > Since SIGCANCEL and SIGTIMER are the same, I wondered whether this > change impacts timer_create. > > Here is what i found: timer_create arranges for SIGCANCEL/SIGTIMER to be > sent to the internal helper thread, which does this: > > /* sigwaitinfo cannot be used here, since it deletes > SIGCANCEL == SIGTIMER from the set. */ > > /* XXX The size argument hopefully will have to be changed to the > real size of the user-level sigset_t. */ > int result = SYSCALL_CANCEL (rt_sigtimedwait, &ss, &si, NULL, _NSIG / 8); > > if (result > 0) > { > if (si.si_code == SI_TIMER) > { > struct timer *tk = (struct timer *) si.si_ptr; > … > else if (si.si_code == SI_TKILL) > /* The thread is canceled. */ > pthread_exit (NULL); > } > > This suggests to me that the helper thread does NOT depend on EINTR > being generated for SIGCANCEL/SIGTIMER, and it should be fine to use > SA_RESTART for that signal as far as timer_create is concerned. > > If you agree, it probably makes sense to add this bit of information to > the commit message. Ack, this is what I have added: -- As a side note regarding SIGCANCEL and SIGTIMER being the the same, it should not impact timer_create functionality. It arranges for SIGCANCEL/SIGTIMER to be sent to the internal helper thread, which in turn check if the si.si_code is SI_TIMER and call pthread_exit otherwise (sysdeps/unix/sysv/linux/timer_routines.c:129). This suggests that the helper thread does NOT depend on EINTR being generated for SIGCANCEL/SIGTIMER, and it should be fine to use SA_RESTART for that signal as far as timer_create is concerned.