[newlib-cygwin/cygwin-3_6-branch] Cygwin: signal: Do not handle signals while waiting for wakeup evt

Takashi Yano tyan0@sourceware.org
Fri May 2 11:28:46 GMT 2025


https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=39373feb0352d1c4d62670da05ea417a95b7a495

commit 39373feb0352d1c4d62670da05ea417a95b7a495
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Apr 30 23:09:21 2025 +0900

    Cygwin: signal: Do not handle signals while waiting for wakeup evt
    
    ... Otherwise, the opportunity for cleanup the wakeup event handle etc.
    may be lost because the user signal handler never returns if it calls
    longjmp(). This results in handle leak because the wakeup event handle
    will not be closed. This issue happens when the commnad e.g. "stress-ng
    --mprotect 1 -t 5" is executed. Instead, call call_signal_handler()
    after cleaning up if some signals are armed during waiting wakeup event.
    This essentially reverts the commit d243e51ef1d3, however, the deadlock
    fixed by that commit no longer occurs even reverting it for some reason.
    This is probably due to the redesign of the signal queue.
    
    In addition, do not touch "incyg" flag in _cygtls::call_signal_handler()
    because the process is still in the cygwin function when a user signal
    handler is called from the cygwin functions such as cygwait().
    
    Addresses: https://sourceware.org/pipermail/cygwin/2025-March/257726.html
    Fixes: d243e51ef1d3 ("Cygwin: signal: Fix deadlock between main thread and sig thread")
    Fixes: 3a1ccfc8c7e6 ("* exceptions.cc (setup_handler): Remove locked flag.  Use 'incyg' flag and in_exception function to determine when we're in a cygwin function.")
    Reported-by: Christian Franke <Christian.Franke@t-online.de>
    Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
    (cherry picked from commit 68991cda81853ca0a90f558239b08d67a0e5465c)

Diff:
---
 winsup/cygwin/exceptions.cc |  3 ---
 winsup/cygwin/sigproc.cc    | 20 +++++++++++---------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 49fc166ff..d1c98e46f 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1756,7 +1756,6 @@ _cygtls::call_signal_handler ()
 
       int this_errno = saved_errno;
       reset_signal_arrived ();
-      incyg = false;
       current_sig = 0;	/* Flag that we can accept another signal */
 
       /* We have to fetch the original return address from the signal stack
@@ -1869,8 +1868,6 @@ _cygtls::call_signal_handler ()
 	}
       unlock ();
 
-      incyg = true;
-
       set_signal_mask (_my_tls.sigmask, (this_sa_flags & SA_SIGINFO)
 					? context1.uc_sigmask : this_oldmask);
       if (this_errno >= 0)
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index fc28be956..361887981 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -611,7 +611,7 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
   bool communing = si.si_signo == __SIGCOMMUNE;
 
   pack.wakeup = NULL;
-  bool wait_for_completion;
+  bool wait_for_completion = false;
   if (!(its_me = p == NULL || p == myself || p == myself_nowait))
     {
       /* It is possible that the process is not yet ready to receive messages
@@ -762,13 +762,10 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
       memcpy (p, si._si_commune._si_str, n); p += n;
     }
 
-  unsigned cw_mask;
-  cw_mask = pack.si.si_signo == __SIGFLUSHFAST ? 0 : cw_sig_restart;
-
   char mtx_name[MAX_PATH];
   shared_name (mtx_name, "sig_send", p->pid);
   mtx = CreateMutex (&sec_none_nih, FALSE, mtx_name);
-  cygwait (mtx, INFINITE, cw_mask);
+  WaitForSingleObject (mtx, INFINITE);
 
   if (its_me && (si.si_signo == __SIGFLUSHFAST || si.si_signo == __SIGFLUSH))
     {
@@ -791,7 +788,7 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
 	  CloseHandle (mtx);
 	  ResetEvent (sigflush_done_evt);
 	  SetEvent (sigflush_evt);
-	  cygwait (sigflush_done_evt, INFINITE, cw_mask);
+	  WaitForSingleObject (sigflush_done_evt, INFINITE);
 	  rc = 0;
 	  goto out;
 	}
@@ -807,8 +804,8 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
       if (!res || packsize == nb)
 	break;
       ReleaseMutex (mtx);
-      cygwait (NULL, 10, cw_mask);
-      cygwait (mtx, INFINITE, cw_mask);
+      Sleep (10);
+      WaitForSingleObject (mtx, INFINITE);
       res = 0;
     }
   ReleaseMutex (mtx);
@@ -843,7 +840,7 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
   if (wait_for_completion)
     {
       sigproc_printf ("Waiting for pack.wakeup %p", pack.wakeup);
-      rc = cygwait (pack.wakeup, WSSC, cw_mask);
+      rc = WaitForSingleObject (pack.wakeup, WSSC);
       ForceCloseHandle (pack.wakeup);
     }
   else
@@ -874,6 +871,11 @@ out:
     }
   if (pack.wakeup)
     ForceCloseHandle (pack.wakeup);
+
+  /* Handle signals here if it was not handled yet */
+  if (wait_for_completion && pack.si.si_signo != __SIGFLUSHFAST)
+    _my_tls.call_signal_handler ();
+
   if (si.si_signo != __SIGPENDING && si.si_signo != __SIGPENDINGALL)
     /* nothing */;
   else if (!rc)


More information about the Cygwin-cvs mailing list