From owner-freebsd-threads@FreeBSD.ORG Fri Aug 20 08:57:27 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7D54610656A6; Fri, 20 Aug 2010 08:57:27 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id C2B068FC27; Fri, 20 Aug 2010 08:57:26 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id o7K8uLiO026057 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 20 Aug 2010 11:56:21 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id o7K8uL5c020925; Fri, 20 Aug 2010 11:56:21 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id o7K8uLl0020924; Fri, 20 Aug 2010 11:56:21 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 20 Aug 2010 11:56:21 +0300 From: Kostik Belousov To: David Xu Message-ID: <20100820085621.GK2396@deviant.kiev.zoral.com.ua> References: <4C696A96.7020709@freebsd.org> <20100816104303.GP2396@deviant.kiev.zoral.com.ua> <4C6AA092.40708@freebsd.org> <4C6BE0F7.10207@freebsd.org> <20100818100403.GS2396@deviant.kiev.zoral.com.ua> <4C6C6184.9030602@freebsd.org> <20100819083809.GC2396@deviant.kiev.zoral.com.ua> <4C6D6384.7080506@freebsd.org> <20100819144218.GH2396@deviant.kiev.zoral.com.ua> <4C6DD70F.5070803@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="uVi4s/+X6ihGtnOs" Content-Disposition: inline In-Reply-To: <4C6DD70F.5070803@freebsd.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-2.7 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_05, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: freebsd-threads@freebsd.org Subject: Re: PTHREAD_CANCEL_DEFERRED X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Aug 2010 08:57:27 -0000 --uVi4s/+X6ihGtnOs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Aug 20, 2010 at 01:14:55AM +0000, David Xu wrote: > Kostik Belousov wrote: >=20 > > > >What would happen in the following situation: > >thr_wake() is called; > >some syscall is started executing and slept, assume that SA_RESTART is > >for SIGHUP (just an example); > >SIGHUP is sent to the process and the thread is selected > >for delivery, also assume that handler is installed. > > > >As I understand, in this situation, EINTR is returned from syscall. >=20 > Yes, because cancellation should have higher priority over signals, > if signal always causes ERESTART to be returned, and system call > is always restarted, the system call may not return forever because > its event it is waiting never happens, for example, it is reading a byte > from a socket, but it is never available, we end up not being able to > cancel the thread, this is incorrect. Again, this is not exactly what I mean. Assume that the cancellation is not requested at all. I see no reason to return EINTR. As I see, you do not check errno when seeing syscall error return in the committed patch. Anyway, below is mostly cosmetic change to reduce code duplication, saved from the reverted patch of mine. Any objections ? diff --git a/lib/libthr/thread/thr_sig.c b/lib/libthr/thread/thr_sig.c index eed6fdb..115aa65 100644 --- a/lib/libthr/thread/thr_sig.c +++ b/lib/libthr/thread/thr_sig.c @@ -268,23 +268,26 @@ _pthread_sigmask(int how, const sigset_t *set, sigset= _t *oset) =20 __weak_reference(__sigsuspend, sigsuspend); =20 -int -_sigsuspend(const sigset_t * set) +static const sigset_t * +thr_remove_thr_signals(const sigset_t *set, sigset_t *newset) { - sigset_t newset; const sigset_t *pset; - int ret; =20 if (SIGISMEMBER(*set, SIGCANCEL)) { - newset =3D *set; - SIGDELSET(newset, SIGCANCEL); - pset =3D &newset; + *newset =3D *set; + SIGDELSET(*newset, SIGCANCEL); + pset =3D newset; } else pset =3D set; + return (pset); +} =20 - ret =3D __sys_sigsuspend(pset); +int +_sigsuspend(const sigset_t * set) +{ + sigset_t newset; =20 - return (ret); + return (__sys_sigsuspend(thr_remove_thr_signals(set, &newset))); } =20 int @@ -292,18 +295,10 @@ __sigsuspend(const sigset_t * set) { struct pthread *curthread =3D _get_curthread(); sigset_t newset; - const sigset_t *pset; int ret; =20 - if (SIGISMEMBER(*set, SIGCANCEL)) { - newset =3D *set; - SIGDELSET(newset, SIGCANCEL); - pset =3D &newset; - } else - pset =3D set; - _thr_cancel_enter(curthread); - ret =3D __sys_sigsuspend(pset); + ret =3D __sys_sigsuspend(thr_remove_thr_signals(set, &newset)); _thr_cancel_leave(curthread); =20 return (ret); @@ -318,17 +313,9 @@ _sigtimedwait(const sigset_t *set, siginfo_t *info, const struct timespec * timeout) { sigset_t newset; - const sigset_t *pset; - int ret; =20 - if (SIGISMEMBER(*set, SIGCANCEL)) { - newset =3D *set; - SIGDELSET(newset, SIGCANCEL); - pset =3D &newset; - } else - pset =3D set; - ret =3D __sys_sigtimedwait(pset, info, timeout); - return (ret); + return (__sys_sigtimedwait(thr_remove_thr_signals(set, &newset), info, + timeout)); } =20 /* @@ -342,17 +329,11 @@ __sigtimedwait(const sigset_t *set, siginfo_t *info, { struct pthread *curthread =3D _get_curthread(); sigset_t newset; - const sigset_t *pset; int ret; =20 - if (SIGISMEMBER(*set, SIGCANCEL)) { - newset =3D *set; - SIGDELSET(newset, SIGCANCEL); - pset =3D &newset; - } else - pset =3D set; _thr_cancel_enter_defer(curthread, 1); - ret =3D __sys_sigtimedwait(pset, info, timeout); + ret =3D __sys_sigtimedwait(thr_remove_thr_signals(set, &newset), info, + timeout); _thr_cancel_leave_defer(curthread, (ret =3D=3D -1)); return (ret); } @@ -361,18 +342,8 @@ int _sigwaitinfo(const sigset_t *set, siginfo_t *info) { sigset_t newset; - const sigset_t *pset; - int ret; - - if (SIGISMEMBER(*set, SIGCANCEL)) { - newset =3D *set; - SIGDELSET(newset, SIGCANCEL); - pset =3D &newset; - } else - pset =3D set; =20 - ret =3D __sys_sigwaitinfo(pset, info); - return (ret); + return (__sys_sigwaitinfo(thr_remove_thr_signals(set, &newset), info)); } =20 /* @@ -385,18 +356,10 @@ __sigwaitinfo(const sigset_t *set, siginfo_t *info) { struct pthread *curthread =3D _get_curthread(); sigset_t newset; - const sigset_t *pset; int ret; =20 - if (SIGISMEMBER(*set, SIGCANCEL)) { - newset =3D *set; - SIGDELSET(newset, SIGCANCEL); - pset =3D &newset; - } else - pset =3D set; - _thr_cancel_enter_defer(curthread, 1); - ret =3D __sys_sigwaitinfo(pset, info); + ret =3D __sys_sigwaitinfo(thr_remove_thr_signals(set, &newset), info); _thr_cancel_leave_defer(curthread, ret =3D=3D -1); return (ret); } @@ -405,18 +368,8 @@ int _sigwait(const sigset_t *set, int *sig) { sigset_t newset; - const sigset_t *pset; - int ret; - - if (SIGISMEMBER(*set, SIGCANCEL)) { - newset =3D *set; - SIGDELSET(newset, SIGCANCEL); - pset =3D &newset; - } else=20 - pset =3D set; =20 - ret =3D __sys_sigwait(pset, sig); - return (ret); + return (__sys_sigwait(thr_remove_thr_signals(set, &newset), sig)); } =20 /* @@ -429,18 +382,10 @@ __sigwait(const sigset_t *set, int *sig) { struct pthread *curthread =3D _get_curthread(); sigset_t newset; - const sigset_t *pset; int ret; =20 - if (SIGISMEMBER(*set, SIGCANCEL)) { - newset =3D *set; - SIGDELSET(newset, SIGCANCEL); - pset =3D &newset; - } else=20 - pset =3D set; - _thr_cancel_enter_defer(curthread, 1); - ret =3D __sys_sigwait(pset, sig); + ret =3D __sys_sigwait(thr_remove_thr_signals(set, &newset), sig); _thr_cancel_leave_defer(curthread, (ret !=3D 0)); return (ret); } --uVi4s/+X6ihGtnOs Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (FreeBSD) iEYEARECAAYFAkxuQzUACgkQC3+MBN1Mb4hIPACfSm79lI2SSygTCWHnVSK0q7bo PDgAoJL5XeXFDwwZ5lO2llP2J9hfsaid =Wt6B -----END PGP SIGNATURE----- --uVi4s/+X6ihGtnOs--