Date: Fri, 20 Aug 2010 11:56:21 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: David Xu <davidxu@freebsd.org> Cc: freebsd-threads@freebsd.org Subject: Re: PTHREAD_CANCEL_DEFERRED Message-ID: <20100820085621.GK2396@deviant.kiev.zoral.com.ua> In-Reply-To: <4C6DD70F.5070803@freebsd.org> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100820085621.GK2396>