DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id
	:list-unsubscribe:list-subscribe:list-archive:list-post
	:list-help:sender:date:from:to:subject:message-id:reply-to
	:references:mime-version:content-type:in-reply-to; q=dns; s=
	default; b=Dr4bh0xcetpp+sME6GwEzOkqQukKb0eN3hQKvKJXiyztoGKRFnIgm
	NEJJlqXBrJTcGYjBdMlUQdVE8HJLO/5IEQrnzS8QN85tnKS9W/Va8thUKO18pnBL
	EKDMomoNTs1J2xeghneSw94v/pgeTdFDJX0b3Oa7jd5pa2uIQAkbmA=
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Date: Mon, 23 Feb 2015 15:03:17 +0100
From: Corinna Vinschen <corinna-cygwin AT cygwin DOT com>
To: cygwin AT cygwin DOT com
Subject: Re: Unexpected EINVAL from pthread_join
Message-ID: <20150223140317.GP437@calimero.vinschen.de>
Reply-To: cygwin AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
References: <20150222225437 DOT 271e929b AT tukaani DOT org> <20150223121445 DOT GL437 AT calimero DOT vinschen DOT de> <20150223125914 DOT GO437 AT calimero DOT vinschen DOT de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;	protocol="application/pgp-signature"; boundary="6sj9mcRtP+pTWLOo"
Content-Disposition: inline
In-Reply-To: <20150223125914.GO437@calimero.vinschen.de>
User-Agent: Mutt/1.5.23 (2014-03-12)

--6sj9mcRtP+pTWLOo
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Feb 23 13:59, Corinna Vinschen wrote:
> On Feb 23 13:14, Corinna Vinschen wrote:
> > On Feb 22 22:54, Lasse Collin wrote:
> > > It seems that a signal can cause pthread_join to incorrectly return
> > > EINVAL. I debugged it only a little but hopefully someone finds this
> > > useful:
> > >=20
> > > In the file thread.cc, function pthread::join, the call to cygwait may
> > > return WAIT_SIGNALED if a signal is sent to the process. The switch
> > > statement handling the return value assumes that only WAIT_OBJECT_0 a=
nd
> > > WAIT_CANCELED are possible. The default section of the switch stateme=
nt
> > > has a comment "should never happen" and it returns EINVAL. It might be
> > > that the problem occurs only when SA_RESTART isn't used.
> [...]
> I looked into the Linux man page for pthread_join(1).  It doesn't mention
> signals and EINTR at all.  Then I looked into the SUSv4 pages(2) and it
> only has this to say:
>=20
>   The pthread_join() function shall not return an error code of [EINTR].
>=20
> Searching further on this I found this(3):
>=20
>   The wait in pthread_join is not broken by a signal. If a thread
>   waiting in pthread_join receives a signal that is not masked, if will
>   execute the signal handler, and then return to waiting in
>   pthread_join.
>=20
> Taking that at face value, the following patch should do the right
> thing, doesn't it?

On second thought, this is not the right way to handle this.  The
WAIT_SIGNALED is returned because we're in the main thread and
SA_RESTART is not set, as you assumed above.  This leads to the
question why this scenario isn't handled directly in cygwait.

So what I did now is to apply the below patch to CVS.  It adds a flag
cw_sig_restart to cygwait, which also restarts in the main thread if
SA_RESTART is not set, as it's supposed to be for pthread_join.

I uploaded a new developer snapshot to https://cygwin.com/snapshots/
Can you please test if it works as desired?


Thanks,
Corinna


        * cygwait.h (enum cw_wait_mask): Add cw_sig_restart.  Add comments
        to explain the meaning of the possible values.
        * cygwait.cc (is_cw_sig_restart): Define.
        (is_cw_sig_handle): Check for cw_sig_restart as well.
        (cygwait): Restart always if cw_sig_restart is set.
        * thread.cc (pthread::join): Call cygwait with cw_sig_restart flag
        to avoid having to handle signals at all.

Index: cygwait.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/src/src/winsup/cygwin/cygwait.h,v
retrieving revision 1.10
diff -u -p -r1.10 cygwait.h
--- cygwait.h	9 Apr 2013 01:01:19 -0000	1.10
+++ cygwait.h	23 Feb 2015 13:54:48 -0000
@@ -1,7 +1,7 @@
 /* cygwait.h
=20
-   Copyright 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2=
013
-   Red Hat, Inc.
+   Copyright 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2=
013,
+   2015 Red Hat, Inc.
=20
    This file is part of Cygwin.
=20
@@ -16,11 +16,12 @@
=20
 enum cw_wait_mask
 {
-  cw_cancel =3D		0x0001,
-  cw_cancel_self =3D	0x0002,
-  cw_sig =3D		0x0004,
-  cw_sig_eintr =3D	0x0008,
-  cw_sig_cont =3D		0x0010
+  cw_cancel =3D		0x0001,	/* Cancellation point.  Return to caller. */
+  cw_cancel_self =3D	0x0002,	/* Cancellation point.  Cancel self. */
+  cw_sig =3D		0x0004,	/* Handle signals. */
+  cw_sig_eintr =3D	0x0008,	/* Caller handles signals. */
+  cw_sig_cont =3D		0x0010,	/* Caller handles SIGCONT. */
+  cw_sig_restart =3D	0x0020	/* Restart even if SA_RESTART isn't set. */
 };
=20
 extern LARGE_INTEGER cw_nowait_storage;
Index: cygwait.cc
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/src/src/winsup/cygwin/cygwait.cc,v
retrieving revision 1.12
diff -u -p -r1.12 cygwait.cc
--- cygwait.cc	23 Feb 2015 13:32:16 -0000	1.12
+++ cygwait.cc	23 Feb 2015 13:54:48 -0000
@@ -18,8 +18,10 @@
 #define is_cw_sig		(mask & cw_sig)
 #define is_cw_sig_eintr		(mask & cw_sig_eintr)
 #define is_cw_sig_cont		(mask & cw_sig_cont)
+#define is_cw_sig_restart	(mask & cw_sig_restart)
=20
-#define is_cw_sig_handle	(mask & (cw_sig | cw_sig_eintr | cw_sig_cont))
+#define is_cw_sig_handle	(mask & (cw_sig | cw_sig_eintr \
+					 | cw_sig_cont | cw_sig_restart))
=20
 LARGE_INTEGER cw_nowait_storage;
=20
@@ -88,7 +90,7 @@ cygwait (HANDLE object, PLARGE_INTEGER t
 	    continue;
 	  if (is_cw_sig_eintr || (is_cw_sig_cont && sig =3D=3D SIGCONT))
 	    ;
-	  else if (_my_tls.call_signal_handler ())
+	  else if (_my_tls.call_signal_handler () || is_cw_sig_restart)
 	    continue;
 	  res =3D WAIT_SIGNALED;	/* caller will deal with signals */
 	}
Index: thread.cc
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/src/src/winsup/cygwin/thread.cc,v
retrieving revision 1.296
diff -u -p -r1.296 thread.cc
--- thread.cc	28 Nov 2014 20:46:13 -0000	1.296
+++ thread.cc	23 Feb 2015 13:54:48 -0000
@@ -1,7 +1,7 @@
 /* thread.cc: Locking and threading module functions
=20
    Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2=
008,
-   2009, 2010, 2011, 2012, 2013, 2014 Red Hat, Inc.
+   2009, 2010, 2011, 2012, 2013, 2014, 2015 Red Hat, Inc.
=20
 This file is part of Cygwin.
=20
@@ -2399,7 +2399,8 @@ pthread::join (pthread_t *thread, void *
       (*thread)->attr.joinable =3D PTHREAD_CREATE_DETACHED;
       (*thread)->mutex.unlock ();
=20
-      switch (cygwait ((*thread)->win32_obj_id, cw_infinite, cw_sig | cw_c=
ancel))
+      switch (cygwait ((*thread)->win32_obj_id, cw_infinite,
+		       cw_sig | cw_sig_restart | cw_cancel))
 	{
 	case WAIT_OBJECT_0:
 	  if (return_val)


--=20
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

--6sj9mcRtP+pTWLOo
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJU6zMlAAoJEPU2Bp2uRE+gZ6oP/AhYAZUQNsBPd8JFh0wLGcfn
1B00ry1bSXi0zh8HKGzLiSkOb2WjO50mHMmxJcwRNfny68cTmOIH8wcK1vBQllXE
9NFUWDX4t9zlrtZ2r4oswsUFpUSgCKtUwA/heXz/TxM+wRaPA/RAtmlTSg6AbYA+
0RBPNKvvdLMHrIMzCu5vGwE8vY6G0QwUapnOgj7x7hHPNzyvW3Xwbj3tWFeTFra6
7IVUFgdt2SeTxES35tbHrGyTJKnKrVkcOKBXQQPYqZ9wNjkLdRQKhrIt9DlLFukd
tMBlzx8mJE5+Q+mHO9BJlOrcjNAqsKjn7AAXwIwRrljAnscZzW5TdYzF8AN6x42h
SYSQBupM7oDPpLxenY4VtKZ9bSrzt4XiDkNKcyPZNvg7ILU13LoC0md8DOwNvxUN
a7c1r5LX5ymRoZLxNqVWcFgn8vKF0KQDB4dCPlXnEIiHpyg5tYDKmgciFSbHNV+c
rabMIcs/o476q1hhTrQXRqXmt0oITZhpDWwZtGfD3IAN4s/Qyhg7RexDBNuC1FXz
9NikP+3qxjr4tGklYLTIUhpBAV4YC7ndikgPrjVdGFATaAk+ekhLwFQ2ID51iHBe
ysgVGXgNouE8HpmXAJXdLnwAlqGlLEgWr6OAbP67ih4bmC19AhT2QC2Y2TVMPdA6
XPbERP5Lj5QrIYjo3u+D
=xp57
-----END PGP SIGNATURE-----

--6sj9mcRtP+pTWLOo--