Unexpected EINVAL from pthread_join
Corinna Vinschen
corinna-cygwin@cygwin.com
Mon Feb 23 15:25:00 GMT 2015
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:
> > >
> > > 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 and
> > > WAIT_CANCELED are possible. The default section of the switch statement
> > > 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:
>
> The pthread_join() function shall not return an error code of [EINTR].
>
> Searching further on this I found this(3):
>
> 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.
>
> 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
===================================================================
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
- Copyright 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013
- Red Hat, Inc.
+ Copyright 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013,
+ 2015 Red Hat, Inc.
This file is part of Cygwin.
@@ -16,11 +16,12 @@
enum cw_wait_mask
{
- cw_cancel = 0x0001,
- cw_cancel_self = 0x0002,
- cw_sig = 0x0004,
- cw_sig_eintr = 0x0008,
- cw_sig_cont = 0x0010
+ cw_cancel = 0x0001, /* Cancellation point. Return to caller. */
+ cw_cancel_self = 0x0002, /* Cancellation point. Cancel self. */
+ cw_sig = 0x0004, /* Handle signals. */
+ cw_sig_eintr = 0x0008, /* Caller handles signals. */
+ cw_sig_cont = 0x0010, /* Caller handles SIGCONT. */
+ cw_sig_restart = 0x0020 /* Restart even if SA_RESTART isn't set. */
};
extern LARGE_INTEGER cw_nowait_storage;
Index: cygwait.cc
===================================================================
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)
-#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))
LARGE_INTEGER cw_nowait_storage;
@@ -88,7 +90,7 @@ cygwait (HANDLE object, PLARGE_INTEGER t
continue;
if (is_cw_sig_eintr || (is_cw_sig_cont && sig == SIGCONT))
;
- else if (_my_tls.call_signal_handler ())
+ else if (_my_tls.call_signal_handler () || is_cw_sig_restart)
continue;
res = WAIT_SIGNALED; /* caller will deal with signals */
}
Index: thread.cc
===================================================================
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
Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008,
- 2009, 2010, 2011, 2012, 2013, 2014 Red Hat, Inc.
+ 2009, 2010, 2011, 2012, 2013, 2014, 2015 Red Hat, Inc.
This file is part of Cygwin.
@@ -2399,7 +2399,8 @@ pthread::join (pthread_t *thread, void *
(*thread)->attr.joinable = PTHREAD_CREATE_DETACHED;
(*thread)->mutex.unlock ();
- switch (cygwait ((*thread)->win32_obj_id, cw_infinite, cw_sig | cw_cancel))
+ switch (cygwait ((*thread)->win32_obj_id, cw_infinite,
+ cw_sig | cw_sig_restart | cw_cancel))
{
case WAIT_OBJECT_0:
if (return_val)
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin/attachments/20150223/39040dd5/attachment.sig>
More information about the Cygwin
mailing list