[newlib-cygwin] Cygwin: select: revamp non-polling code for signalfd

2019-08-18 Thread Corinna Vinschen
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=7097b05eda2f8e9058eab4fda8dedacdfb7ffd7f

commit 7097b05eda2f8e9058eab4fda8dedacdfb7ffd7f
Author: Corinna Vinschen 
Date:   Fri Aug 16 16:36:06 2019 +0200

Cygwin: select: revamp non-polling code for signalfd

Rather than waiting for signalfd_select_wait in a thread, which is racy,
create a global event "my_pendingsigs_evt" which is set and reset by
wait_sig depending only on the fact if blocked signals are pending or not.

This in turn allows to WFMO on this event in select as soon as signalfds
are present in the read descriptor set.  Select's peek and verify
will then check if one of the present signalfds is affected.

Signed-off-by: Corinna Vinschen 

Diff:
---
 winsup/cygwin/cygtls.h   |  2 +-
 winsup/cygwin/exceptions.cc  |  8 -
 winsup/cygwin/select.cc  | 80 +++-
 winsup/cygwin/select.h   | 11 ++
 winsup/cygwin/signal.cc  |  1 -
 winsup/cygwin/sigproc.cc | 17 ++
 winsup/cygwin/tlsoffsets.h   | 16 -
 winsup/cygwin/tlsoffsets64.h | 16 -
 8 files changed, 40 insertions(+), 111 deletions(-)

diff --git a/winsup/cygwin/cygtls.h b/winsup/cygwin/cygtls.h
index 65a905c..a2e3676 100644
--- a/winsup/cygwin/cygtls.h
+++ b/winsup/cygwin/cygtls.h
@@ -189,8 +189,8 @@ public:
   stack_t altstack;
   siginfo_t *sigwait_info;
   HANDLE signal_arrived;
-  HANDLE signalfd_select_wait;
   bool will_wait_for_signal;
+  long __align;/* Needed to align context to 16 byte. 
*/
   /* context MUST be aligned to 16 byte, otherwise RtlCaptureContext fails.
  If you prepend cygtls members here, make sure context stays 16 byte
  aligned. */
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 1765f43..848f9bd 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1469,14 +1469,6 @@ sigpacket::process ()
   if (issig_wait)
 {
   tls->sigwait_mask = 0;
-  /* If the catching thread is running select on a signalfd, don't call
-the signal handler and don't remove the signal from the queue. */
-  if (tls->signalfd_select_wait)
-   {
- SetEvent (tls->signalfd_select_wait);
- rc = 0;
- goto done;
-   }
   goto dosig;
 }
 
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 4e9256b..ac73e0a 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -1849,80 +1849,6 @@ fhandler_windows::select_except (select_stuff *ss)
   return s;
 }
 
-static int start_thread_signalfd (select_record *, select_stuff *);
-
-static DWORD WINAPI
-thread_signalfd (void *arg)
-{
-  select_signalfd_info *si = (select_signalfd_info *) arg;
-  bool event = false;
-
-  while (!event)
-{
-  sigset_t set = 0;
-  _cygtls *tls = si->start->tls;
-
-  for (select_record *s = si->start; (s = s->next); )
-   if (s->startup == start_thread_signalfd)
- set |= ((fhandler_signalfd *) s->fh)->get_sigset ();
-  set_signal_mask (tls->sigwait_mask, set);
-  tls->signalfd_select_wait = si->evt;
-  sig_dispatch_pending (true);
-  switch (WaitForSingleObject (si->evt, INFINITE))
-   {
-   case WAIT_OBJECT_0:
- tls->signalfd_select_wait = NULL;
- event = true;
- break;
-   default:
- break;
-   }
-  if (si->stop_thread)
-   break;
-  if (!event)
-   Sleep (1L);
-}
-  CloseHandle (si->evt);
-  return 0;
-}
-
-static int
-start_thread_signalfd (select_record *me, select_stuff *stuff)
-{
-  select_signalfd_info *si;
-
-  if ((si = stuff->device_specific_signalfd))
-{
-  me->h = *si->thread;
-  return 1;
-}
-  si = new select_signalfd_info;
-  si->start = >start;
-  si->stop_thread = false;
-  si->evt = CreateEventW (_none_nih, TRUE, FALSE, NULL);
-  si->thread = new cygthread (thread_signalfd, si, "signalfdsel");
-  me->h = *si->thread;
-  stuff->device_specific_signalfd = si;
-  return 1;
-}
-
-static void
-signalfd_cleanup (select_record *, select_stuff *stuff)
-{
-  select_signalfd_info *si;
-
-  if (!(si = stuff->device_specific_signalfd))
-return;
-  if (si->thread)
-{
-  si->stop_thread = true;
-  SetEvent (si->evt);
-  si->thread->detach ();
-}
-  delete si;
-  stuff->device_specific_signalfd = NULL;
-}
-
 static int
 peek_signalfd (select_record *me, bool)
 {
@@ -1942,17 +1868,19 @@ verify_signalfd (select_record *me, fd_set *rfds, 
fd_set *wfds, fd_set *efds)
   return peek_signalfd (me, true);
 }
 
+extern HANDLE my_pendingsigs_evt;
+
 select_record *
 fhandler_signalfd::select_read (select_stuff *stuff)
 {
   select_record *s = stuff->start.next;
   if (!s->startup)
 {
-  s->startup = start_thread_signalfd;
+  s->startup = no_startup;
   s->verify = verify_signalfd;
-  s->cleanup = signalfd_cleanup;
 }
   s->peek = peek_signalfd;
+  s->h = 

[newlib-cygwin] Revert "Cygwin: fix potential SEGV in sigwaitinfo/signalfd scenario"

2019-08-18 Thread Corinna Vinschen
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=b7399d5e6f8ad5b15cd725f66b3e49732393ef03

commit b7399d5e6f8ad5b15cd725f66b3e49732393ef03
Author: Corinna Vinschen 
Date:   Fri Aug 16 16:36:20 2019 +0200

Revert "Cygwin: fix potential SEGV in sigwaitinfo/signalfd scenario"

This reverts commit 92115a83a4579635e253be2887d3706d28b477fd.

This was utterly wrong.

Diff:
---
 winsup/cygwin/exceptions.cc | 17 +++--
 winsup/cygwin/release/3.1.0 |  3 ---
 2 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 9bdf9f0..1765f43 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1628,7 +1628,7 @@ _cygtls::call_signal_handler ()
   if (retaddr () == (__tlsstack_t) sigdelayed)
pop ();
 
-  debug_only_printf ("dealing with signal %d, handler %p", sig, func);
+  debug_only_printf ("dealing with signal %d", sig);
   this_sa_flags = sa_flags;
 
   sigset_t this_oldmask = set_process_mask_delta ();
@@ -1647,12 +1647,8 @@ _cygtls::call_signal_handler ()
 
   ucontext_t *thiscontext = NULL, context_copy;
 
-  /* Only make a context for SA_SIGINFO handlers, only if a handler
- exists.  If handler is NULL, drop SA_SIGINFO flag to avoid
-accidental context access later in the function. */
-  if (!thisfunc)
-   this_sa_flags &= ~SA_SIGINFO;
-  else if (this_sa_flags & SA_SIGINFO)
+  /* Only make a context for SA_SIGINFO handlers */
+  if (this_sa_flags & SA_SIGINFO)
{
  context.uc_link = 0;
  context.uc_flags = 0;
@@ -1714,11 +1710,6 @@ _cygtls::call_signal_handler ()
   sig = 0; /* Flag that we can accept another signal */
   unlock ();   /* unlock signal stack */
 
-  /* Handler may be NUll in sigwaitinfo/signalfd scenario.  Avoid
-crashing by calling a NULL function. */
-  if (!thisfunc)
-   goto skip_calling_handler;
-
   /* Alternate signal stack requested for this signal and alternate signal
 stack set up for this thread? */
   if (this_sa_flags & SA_ONSTACK
@@ -1826,8 +1817,6 @@ _cygtls::call_signal_handler ()
   signal handler. */
thisfunc (thissig, , thiscontext);
 
-skip_calling_handler:
-
   incyg = true;
 
   set_signal_mask (_my_tls.sigmask, (this_sa_flags & SA_SIGINFO)
diff --git a/winsup/cygwin/release/3.1.0 b/winsup/cygwin/release/3.1.0
index c9cb7c0..ccb63c3 100644
--- a/winsup/cygwin/release/3.1.0
+++ b/winsup/cygwin/release/3.1.0
@@ -71,6 +71,3 @@ Bug Fixes
 - 64 bit only: Avoid collisions between memory maps created with shmat
   and Windows datastructures during fork.
   Addresses: https://cygwin.com/ml/cygwin/2019-08/msg00107.html
-
-- Avoid a SEGV after using signalfd.
-  Addresses: https://cygwin.com/ml/cygwin/2019-08/msg00148.html