Fix race condition when waiting for a signal

* cygtls.h (_cygtls::wait_signal_arrived): Renamed from
        set_signal_arrived.
        (_cygtls::set_signal_arrived): New function signalling signal_arrived.
        (_cygtls::reset_signal_arrived): Don't reset will_wait_for_signal.
        (_cygtls::unwait_signal_arrived): New function only resetting
        will_wait_for_signal.
        (class wait_signal_arrived): Rename from set_signal_arrived.
        Accommodate name change throughout Cygwin.
        (wait_signal_arrived::~wait_signal_arrived): Call
        _cygtls::unwait_signal_arrived.  Add comment.
        * cygserver_ipc.h (ipc_set_proc_info): Fetch signal_arrived handle
        via call to _cygtls::get_signal_arrived.
        * exceptions.cc (_cygtls::interrupt_setup): Signal signal_arrived via
        call to _cygtls::set_signal_arrived.
        (_cygtls::handle_SIGCONT): Ditto.
        * fhandler_socket.cc (fhandler_socket::wait_for_events): Generate
        WSAEVENT array prior to entering wait loop.  Add cancel event object
        if available.  Remove calls to pthread_testcancel and just call
        pthread::static_cancel_self if the cancel event object is signalled.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
This commit is contained in:
Corinna Vinschen 2015-11-27 14:39:11 +01:00
parent 9471b0b36d
commit c43e9340f1
12 changed files with 63 additions and 26 deletions

View File

@ -1,3 +1,25 @@
2015-11-27 Corinna Vinschen <corinna@vinschen.de>
* cygtls.h (_cygtls::wait_signal_arrived): Renamed from
set_signal_arrived.
(_cygtls::set_signal_arrived): New function signalling signal_arrived.
(_cygtls::reset_signal_arrived): Don't reset will_wait_for_signal.
(_cygtls::unwait_signal_arrived): New function only resetting
will_wait_for_signal.
(class wait_signal_arrived): Rename from set_signal_arrived.
Accommodate name change throughout Cygwin.
(wait_signal_arrived::~wait_signal_arrived): Call
_cygtls::unwait_signal_arrived. Add comment.
* cygserver_ipc.h (ipc_set_proc_info): Fetch signal_arrived handle
via call to _cygtls::get_signal_arrived.
* exceptions.cc (_cygtls::interrupt_setup): Signal signal_arrived via
call to _cygtls::set_signal_arrived.
(_cygtls::handle_SIGCONT): Ditto.
* fhandler_socket.cc (fhandler_socket::wait_for_events): Generate
WSAEVENT array prior to entering wait loop. Add cancel event object
if available. Remove calls to pthread_testcancel and just call
pthread::static_cancel_self if the cancel event object is signalled.
2015-11-26 Corinna Vinschen <corinna@vinschen.de>
* path.cc (symlink_native): Fix index when looking for colon in path.

View File

@ -43,10 +43,7 @@ ipc_set_proc_info (proc &blk, bool in_fork = false)
blk.gidcnt = 0;
blk.gidlist = NULL;
blk.is_admin = false;
if (in_fork)
blk.signal_arrived = NULL;
else
_my_tls.set_signal_arrived (true, blk.signal_arrived);
blk.signal_arrived = in_fork ? NULL : _my_tls.get_signal_arrived (true);
}
#endif /* __INSIDE_CYGWIN__ */

View File

@ -1,7 +1,7 @@
/* cygthread.cc
Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2008, 2009,
2010, 2011, 2012, 2013 Red Hat, Inc.
2010, 2011, 2012, 2013, 2015 Red Hat, Inc.
This software is a copyrighted work licensed under the terms of the
Cygwin license. Please consult the file "CYGWIN_LICENSE" for
@ -374,7 +374,7 @@ cygthread::detach (HANDLE sigwait)
unsigned n = 2;
DWORD howlong = INFINITE;
w4[0] = sigwait;
set_signal_arrived here (w4[1]);
wait_signal_arrived here (w4[1]);
/* For a description of the below loop see the end of this file */
for (int i = 0; i < 2; i++)
switch (res = WaitForMultipleObjects (n, w4, FALSE, howlong))

View File

@ -248,7 +248,7 @@ public:
}
return signal_arrived;
}
void set_signal_arrived (bool setit, HANDLE& h)
void wait_signal_arrived (bool setit, HANDLE& h)
{
if (!setit)
will_wait_for_signal = false;
@ -258,10 +258,17 @@ public:
will_wait_for_signal = true;
}
}
void set_signal_arrived ()
{
SetEvent (get_signal_arrived (false));
}
void reset_signal_arrived ()
{
if (signal_arrived)
ResetEvent (signal_arrived);
}
void unwait_signal_arrived ()
{
will_wait_for_signal = false;
}
void handle_SIGCONT ();
@ -430,14 +437,18 @@ public:
}
#endif /* __x86_64__ */
class set_signal_arrived
class wait_signal_arrived
{
public:
set_signal_arrived (bool setit, HANDLE& h) { _my_tls.set_signal_arrived (setit, h); }
set_signal_arrived (HANDLE& h) { _my_tls.set_signal_arrived (true, h); }
wait_signal_arrived (bool setit, HANDLE& h) { _my_tls.wait_signal_arrived (setit, h); }
wait_signal_arrived (HANDLE& h) { _my_tls.wait_signal_arrived (true, h); }
operator int () const {return _my_tls.will_wait_for_signal;}
~set_signal_arrived () { _my_tls.reset_signal_arrived (); }
/* Do not reset the signal_arrived event just because we leave the scope of
this wait_signal_arrived object. This may lead to all sorts of races.
The only method actually resetting the signal_arrived event is
_cygtls::call_signal_handler. */
~wait_signal_arrived () { _my_tls.unwait_signal_arrived (); }
};
#define __getreent() (&_my_tls.local_clib)

View File

@ -40,7 +40,7 @@ cygwait (HANDLE object, PLARGE_INTEGER timeout, unsigned mask)
if (object)
wait_objects[num++] = object;
set_signal_arrived thread_waiting (is_cw_sig_handle, wait_objects[num]);
wait_signal_arrived thread_waiting (is_cw_sig_handle, wait_objects[num]);
debug_only_printf ("object %p, thread waiting %d, signal_arrived %p", object, (int) thread_waiting, _my_tls.signal_arrived);
DWORD sig_n;
if (!thread_waiting)

View File

@ -972,7 +972,7 @@ _cygtls::interrupt_setup (siginfo_t& si, void *handler, struct sigaction& siga)
this->sig = si.si_signo; /* Should always be last thing set to avoid race */
if (incyg)
SetEvent (get_signal_arrived (false));
set_signal_arrived ();
if (!have_execed)
proc_subproc (PROC_CLEARWAIT, 1);
@ -1404,7 +1404,7 @@ _cygtls::handle_SIGCONT ()
else
{
sig = SIGCONT;
SetEvent (signal_arrived); /* alert sig_handle_tty_stop */
set_signal_arrived (); /* alert sig_handle_tty_stop */
sigsent = true;
}
/* Clear pending stop signals */

View File

@ -748,6 +748,12 @@ fhandler_socket::wait_for_events (const long event_mask, const DWORD flags)
int ret;
long events = 0;
WSAEVENT ev[3] = { wsock_evt, NULL, NULL };
wait_signal_arrived here (ev[1]);
DWORD ev_cnt = 2;
if ((ev[2] = pthread::get_cancel_event ()) != NULL)
++ev_cnt;
while (!(ret = evaluate_events (event_mask, events, !(flags & MSG_PEEK)))
&& !events)
{
@ -757,14 +763,9 @@ fhandler_socket::wait_for_events (const long event_mask, const DWORD flags)
return SOCKET_ERROR;
}
WSAEVENT ev[2] = { wsock_evt };
set_signal_arrived here (ev[1]);
switch (WSAWaitForMultipleEvents (2, ev, FALSE, 50, FALSE))
switch (WSAWaitForMultipleEvents (ev_cnt, ev, FALSE, 50, FALSE))
{
case WSA_WAIT_TIMEOUT:
pthread_testcancel ();
break;
case WSA_WAIT_EVENT_0:
break;
@ -774,8 +775,11 @@ fhandler_socket::wait_for_events (const long event_mask, const DWORD flags)
WSASetLastError (WSAEINTR);
return SOCKET_ERROR;
case WSA_WAIT_EVENT_0 + 2:
pthread::static_cancel_self ();
break;
default:
pthread_testcancel ();
/* wsock_evt can be NULL. We're generating the same errno values
as for sockets on which shutdown has been called. */
if (WSAGetLastError () != WSA_INVALID_HANDLE)

View File

@ -1,7 +1,7 @@
/* fhandler_windows.cc: code to access windows message queues.
Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2009, 2011, 2012,
2013 Red Hat, Inc.
2013, 2015 Red Hat, Inc.
Written by Sergey S. Okhapkin (sos@prospect.com.ru).
Feedback and testing by Andy Piper (andyp@parallax.co.uk).
@ -92,7 +92,7 @@ fhandler_windows::read (void *buf, size_t& len)
}
HANDLE w4[2];
set_signal_arrived here (w4[0]);
wait_signal_arrived here (w4[0]);
DWORD cnt = 1;
if ((w4[1] = pthread::get_cancel_event ()) != NULL)
++cnt;

View File

@ -1300,7 +1300,7 @@ lf_setlock (lockf_t *lock, inode_t *node, lockf_t **clean, HANDLE fhdl)
timeout = 100L;
DWORD WAIT_SIGNAL_ARRIVED = WAIT_OBJECT_0 + wait_count;
set_signal_arrived here (w4[wait_count++]);
wait_signal_arrived here (w4[wait_count++]);
DWORD WAIT_THREAD_CANCELED = WAIT_TIMEOUT + 1;
HANDLE cancel_event = pthread::get_cancel_event ();

View File

@ -178,7 +178,7 @@ ipc_cond_timedwait (HANDLE evt, HANDLE mtx, const struct timespec *abstime)
DWORD timer_idx = 0;
int ret = 0;
set_signal_arrived here (w4[1]);
wait_signal_arrived here (w4[1]);
if ((w4[cnt] = pthread::get_cancel_event ()) != NULL)
++cnt;
if (abstime)

View File

@ -41,3 +41,6 @@ Bug Fixes
- Replaced old, buggy strtold implementation with well-tested gdtoa version
from David M. Gay.
Addresses: https://cygwin.com/ml/cygwin/2015-11/msg00205.html
- Fix a race condition in signal handling.
Addresses: https://cygwin.com/ml/cygwin/2015-11/msg00387.html

View File

@ -353,7 +353,7 @@ select_stuff::wait (fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
select_record *s = &start;
DWORD m = 0;
set_signal_arrived here (w4[m++]);
wait_signal_arrived here (w4[m++]);
if ((w4[m] = pthread::get_cancel_event ()) != NULL)
m++;