]> sourceware.org Git - newlib-cygwin.git/commitdiff
Fix race condition when waiting for a signal
authorCorinna Vinschen <corinna@vinschen.de>
Fri, 27 Nov 2015 13:39:11 +0000 (14:39 +0100)
committerCorinna Vinschen <corinna@vinschen.de>
Fri, 27 Nov 2015 13:39:11 +0000 (14:39 +0100)
        * 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>
12 files changed:
winsup/cygwin/ChangeLog
winsup/cygwin/cygserver_ipc.h
winsup/cygwin/cygthread.cc
winsup/cygwin/cygtls.h
winsup/cygwin/cygwait.cc
winsup/cygwin/exceptions.cc
winsup/cygwin/fhandler_socket.cc
winsup/cygwin/fhandler_windows.cc
winsup/cygwin/flock.cc
winsup/cygwin/posix_ipc.cc
winsup/cygwin/release/2.4.0
winsup/cygwin/select.cc

index 0183d82c3e75e36b99010eed188d32713669b03d..474bbc875e6c4b16e0ccd20a0123c0cee8403391 100644 (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.
index 14495bc9421eddb0f485e1fbcce8e47ec138c0ad..9631eacc280192e26e4fe2f9d3f768ac7d29699e 100644 (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__ */
 
index e48a73e545e7ca90884fc891f1b188e0ab3bf863..8597541526e7b6fb5ddd92bbb004fde62d8d022b 100644 (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))
index b386b989dc29b0d1c5ac424f81d7064f1eaeddf4..5271473240fd4d619342ba8a977aa1ee6be38d47 100644 (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)
index 71d30d16419628abda4373328ff87398fdc0a56c..3fc9a38320f215b3f17187bdceed280afc4a7f5e 100644 (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)
index 9119a8c083e289a1ed3eb61d19cfb0b61a7b5d3e..c3a45d2887a486ba78707f6c2399e05c3f2fb405 100644 (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 */
index bc3c610e0a1084b5ef5ce26d5c65d996a19c3030..21bc7316e6f6e6e6d2da85147632242a0545b32d 100644 (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)
index 5cafe13d94c06cec0bdcd7f585474ef7947f8da3..a25812748981adb3be55958b08aefd279bf0d8ff 100644 (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;
index f7c04c8e1156060ad97f76df3c2c56c14cdb9589..d0fea282a575a78a9535d23173aaa6e4b99f1ee3 100644 (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 ();
index ef05dbc1cd8538921ca3018a5283bc4d167f4056..e82c38526966166c230b5c5af1bf4cafe3f80a64 100644 (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)
index bb47216d2a656cd5e4790266abf70f1d1c066d5f..cb1b069cfd67885c5f36c28e7ed216f7b2d3777d 100644 (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
index 803e5d5f56d8bbff3f874a61c2a97e8cf1a11b7a..5409205be8bd4f68aaab7e9427a82f46e23d9a0f 100644 (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++;
 
This page took 0.045897 seconds and 5 git commands to generate.