msys2-runtime: test Takashi's patches

As these are for testing, apply them separately and leave the msys2-runtime PR as a draft for now (msys2/msys2-runtime#253).
jeremyd2019 · Jan 21, 2025 · 284a32a · 284a32a
1 parent f9fe3ae
commit 284a32a
Show file tree

Hide file tree

Showing 5 changed files with 413 additions and 3 deletions.
diff --git a/msys2-runtime/1001-Revert-Cygwin-signal-Do-not-handle-signal-when-__SIG.patch b/msys2-runtime/1001-Revert-Cygwin-signal-Do-not-handle-signal-when-__SIG.patch
@@ -0,0 +1,78 @@
+From 7d863074b54b58caa4696515473e986bbb8d1def Mon Sep 17 00:00:00 2001
+From: Takashi Yano <[email protected]>
+Date: Tue, 21 Jan 2025 12:15:33 +0900
+Subject: [PATCH 1001/1004] Revert "Cygwin: signal: Do not handle signal when
+ __SIGFLUSHFAST is sent"
+
+This reverts commit a22a0ad7c4f0 to apply a new patch for the same
+purpose.
+
+Signed-off-by: Takashi Yano <[email protected]>
+---
+ winsup/cygwin/release/3.5.6 |  5 -----
+ winsup/cygwin/sigproc.cc    | 20 +++++---------------
+ 2 files changed, 5 insertions(+), 20 deletions(-)
+ delete mode 100644 winsup/cygwin/release/3.5.6
+
+diff --git a/winsup/cygwin/release/3.5.6 b/winsup/cygwin/release/3.5.6
+deleted file mode 100644
+index 643d58e585..0000000000
+--- a/winsup/cygwin/release/3.5.6
++++ /dev/null
+@@ -1,5 +0,0 @@
+-Fixes:
+-------
+-
+-- Fix zsh hang at startup.
+-  Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256954.html
+diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
+index c2985277fc..cf43aa9335 100644
+--- a/winsup/cygwin/sigproc.cc
++++ b/winsup/cygwin/sigproc.cc
+@@ -751,14 +751,10 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
+       res = WriteFile (sendsig, leader, packsize, &nb, NULL);
+       if (!res || packsize == nb)
+ 	break;
+-      if (cygwait (NULL, 10, cw_sig_eintr) == WAIT_SIGNALED
+-	  && pack.si.si_signo != __SIGFLUSHFAST)
++      if (cygwait (NULL, 10, cw_sig_eintr) == WAIT_SIGNALED)
+ 	_my_tls.call_signal_handler ();
+       res = 0;
+     }
+-  /* Re-assert signal_arrived which has been cleared in cygwait(). */
+-  if (_my_tls.sig)
+-    _my_tls.set_signal_arrived ();
+
+   if (!res)
+     {
+@@ -789,16 +785,7 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
+   if (wait_for_completion)
+     {
+       sigproc_printf ("Waiting for pack.wakeup %p", pack.wakeup);
+-      do
+-	{
+-	  rc = cygwait (pack.wakeup, WSSC, cw_sig_eintr);
+-	  if (rc == WAIT_SIGNALED && pack.si.si_signo != __SIGFLUSHFAST)
+-	    _my_tls.call_signal_handler ();
+-	}
+-      while (rc != WAIT_OBJECT_0 && rc != WAIT_TIMEOUT);
+-      /* Re-assert signal_arrived which has been cleared in cygwait(). */
+-      if (_my_tls.sig)
+-	_my_tls.set_signal_arrived ();
++      rc = cygwait (pack.wakeup, WSSC);
+       ForceCloseHandle (pack.wakeup);
+     }
+   else
+@@ -819,6 +806,9 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
+       rc = -1;
+     }
+
++  if (wait_for_completion && si.si_signo != __SIGFLUSHFAST)
++    _my_tls.call_signal_handler ();
++
+ out:
+   if (communing && rc)
+     {
+-- 
+2.47.1.windows.2
+
diff --git a/msys2-runtime/1002-Cygwin-cygwait-Make-cygwait-reentrant.patch b/msys2-runtime/1002-Cygwin-cygwait-Make-cygwait-reentrant.patch
@@ -0,0 +1,146 @@
+From 9f9cfe9d6f77684e6fb2b9d6be3e239232e2cc81 Mon Sep 17 00:00:00 2001
+From: Takashi Yano <[email protected]>
+Date: Mon, 20 Jan 2025 23:54:56 +0900
+Subject: [PATCH 1002/1004] Cygwin: cygwait: Make cygwait() reentrant
+
+To allow cygwait() to be called in the signal handler, a locally
+created timer is used instead of _cygtls::locals.cw_timer if it is
+in use.
+
+Co-Authored-By: Corinna Vinschen <[email protected]>
+Signed-off-by: Takashi Yano <[email protected]>
+(cherry picked from commit ea1914000adcbed5c16fc0ba2676173b2f6016c4)
+---
+ winsup/cygwin/cygtls.cc               |  2 ++
+ winsup/cygwin/cygwait.cc              | 22 +++++++++++++++-------
+ winsup/cygwin/local_includes/cygtls.h |  3 ++-
+ winsup/cygwin/select.cc               | 10 +++++++++-
+ 4 files changed, 28 insertions(+), 9 deletions(-)
+
+diff --git a/winsup/cygwin/cygtls.cc b/winsup/cygwin/cygtls.cc
+index afaee8e977..b8b5a01498 100644
+--- a/winsup/cygwin/cygtls.cc
++++ b/winsup/cygwin/cygtls.cc
+@@ -64,6 +64,7 @@ _cygtls::init_thread (void *x, DWORD (*func) (void *, void *))
+   initialized = CYGTLS_INITIALIZED;
+   errno_addr = &(local_clib._errno);
+   locals.cw_timer = NULL;
++  locals.cw_timer_inuse = false;
+   locals.pathbufs.clear ();
+
+   if ((void *) func == (void *) cygthread::stub
+@@ -85,6 +86,7 @@ _cygtls::fixup_after_fork ()
+   signal_arrived = NULL;
+   locals.select.sockevt = NULL;
+   locals.cw_timer = NULL;
++  locals.cw_timer_inuse = false;
+   locals.pathbufs.clear ();
+   wq.thread_ev = NULL;
+ }
+diff --git a/winsup/cygwin/cygwait.cc b/winsup/cygwin/cygwait.cc
+index dbbe1db6e1..bb653f6b7c 100644
+--- a/winsup/cygwin/cygwait.cc
++++ b/winsup/cygwin/cygwait.cc
+@@ -58,16 +58,20 @@ cygwait (HANDLE object, PLARGE_INTEGER timeout, unsigned mask)
+     }
+
+   DWORD timeout_n;
++  HANDLE local_timer = NULL;
++  HANDLE &wait_timer =
++    _my_tls.locals.cw_timer_inuse ? local_timer : _my_tls.locals.cw_timer;
+   if (!timeout)
+     timeout_n = WAIT_TIMEOUT + 1;
+   else
+     {
++      if (!_my_tls.locals.cw_timer_inuse)
++	_my_tls.locals.cw_timer_inuse = true;
+       timeout_n = WAIT_OBJECT_0 + num++;
+-      if (!_my_tls.locals.cw_timer)
+-	NtCreateTimer (&_my_tls.locals.cw_timer, TIMER_ALL_ACCESS, NULL,
+-		       NotificationTimer);
+-      NtSetTimer (_my_tls.locals.cw_timer, timeout, NULL, NULL, FALSE, 0, NULL);
+-      wait_objects[timeout_n] = _my_tls.locals.cw_timer;
++      if (!wait_timer)
++	NtCreateTimer (&wait_timer, TIMER_ALL_ACCESS, NULL, NotificationTimer);
++      NtSetTimer (wait_timer, timeout, NULL, NULL, FALSE, 0, NULL);
++      wait_objects[timeout_n] = wait_timer;
+     }
+
+   while (1)
+@@ -100,7 +104,7 @@ cygwait (HANDLE object, PLARGE_INTEGER timeout, unsigned mask)
+     {
+       TIMER_BASIC_INFORMATION tbi;
+
+-      NtQueryTimer (_my_tls.locals.cw_timer, TimerBasicInformation, &tbi,
++      NtQueryTimer (wait_timer, TimerBasicInformation, &tbi,
+ 		    sizeof tbi, NULL);
+       /* if timer expired, TimeRemaining is negative and represents the
+ 	  system uptime when signalled */
+@@ -108,7 +112,11 @@ cygwait (HANDLE object, PLARGE_INTEGER timeout, unsigned mask)
+ 	timeout->QuadPart = tbi.SignalState || tbi.TimeRemaining.QuadPart < 0LL
+                             ? 0LL : tbi.TimeRemaining.QuadPart;
+       }
+-      NtCancelTimer (_my_tls.locals.cw_timer, NULL);
++      NtCancelTimer (wait_timer, NULL);
++      if (local_timer)
++	NtClose(local_timer);
++      else
++	_my_tls.locals.cw_timer_inuse = false;
+     }
+
+   if (res == WAIT_CANCELED && is_cw_cancel_self)
+diff --git a/winsup/cygwin/local_includes/cygtls.h b/winsup/cygwin/local_includes/cygtls.h
+index e4e3889aff..4bd79c36d7 100644
+--- a/winsup/cygwin/local_includes/cygtls.h
++++ b/winsup/cygwin/local_includes/cygtls.h
+@@ -135,6 +135,7 @@ struct _local_storage
+
+   /* thread.cc */
+   HANDLE cw_timer;
++  bool cw_timer_inuse;
+
+   tls_pathbuf pathbufs;
+   char ttybuf[32];
+@@ -180,7 +181,7 @@ public: /* Do NOT remove this public: line, it's a marker for gentls_offsets. */
+   siginfo_t *sigwait_info;
+   HANDLE signal_arrived;
+   bool will_wait_for_signal;
+-#if 0
++#if 1
+   long __align;			/* Needed to align context to 16 byte. */
+ #endif
+   /* context MUST be aligned to 16 byte, otherwise RtlCaptureContext fails.
+diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
+index bc02c3f9d4..48e811e2a1 100644
+--- a/winsup/cygwin/select.cc
++++ b/winsup/cygwin/select.cc
+@@ -385,10 +385,14 @@ next_while:;
+      to create the timer once per thread.  Since WFMO checks the handles
+      in order, we append the timer as last object, otherwise it's preferred
+      over actual events on the descriptors. */
+-  HANDLE &wait_timer = _my_tls.locals.cw_timer;
++  HANDLE local_timer = NULL;
++  HANDLE &wait_timer =
++    _my_tls.locals.cw_timer_inuse ? local_timer : _my_tls.locals.cw_timer;
+   if (us > 0LL)
+     {
+       NTSTATUS status;
++      if (!_my_tls.locals.cw_timer_inuse)
++	_my_tls.locals.cw_timer_inuse = true;
+       if (!wait_timer)
+ 	{
+ 	  status = NtCreateTimer (&wait_timer, TIMER_ALL_ACCESS, NULL,
+@@ -431,6 +435,10 @@ next_while:;
+     {
+       BOOLEAN current_state;
+       NtCancelTimer (wait_timer, &current_state);
++      if (local_timer)
++	NtClose (local_timer);
++      else
++	_my_tls.locals.cw_timer_inuse = false;
+     }
+
+   wait_states res;
+-- 
+2.47.1.windows.2
+
diff --git a/msys2-runtime/1003-Cygwin-signal-Do-not-handle-signal-when-__SIGFLUSHFA.patch b/msys2-runtime/1003-Cygwin-signal-Do-not-handle-signal-when-__SIGFLUSHFA.patch
@@ -0,0 +1,66 @@
+From adb9c01ea363ca56374e6acd8b3938c82dd2625a Mon Sep 17 00:00:00 2001
+From: Takashi Yano <[email protected]>
+Date: Tue, 21 Jan 2025 00:13:04 +0900
+Subject: [PATCH 1003/1004] Cygwin: signal: Do not handle signal when
+ __SIGFLUSHFAST is sent
+
+The commit a22a0ad7c4f0 was not entirely correct. Even with the patch,
+some hangs still occur. This patch overrides the previous commit along
+with the patch that makes cygwait() reentrant, to fix these hangs.
+
+Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256954.html
+Fixes: d243e51ef1d3 ("Cygwin: signal: Fix deadlock between main thread and sig thread")
+Reported-by: Daisuke Fujimura <[email protected]>
+Reviewed-by: Corinna Vinschen <[email protected]>
+Signed-off-by: Takashi Yano <[email protected]>
+(cherry picked from commit 83afe3e238cd12fb7d4799ba6b3c77e9e3618d91)
+---
+ winsup/cygwin/sigproc.cc | 11 +++++------
+ 1 file changed, 5 insertions(+), 6 deletions(-)
+
+diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
+index cf43aa9335..858a7fd12d 100644
+--- a/winsup/cygwin/sigproc.cc
++++ b/winsup/cygwin/sigproc.cc
+@@ -742,6 +742,9 @@ 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;
++
+   DWORD nb;
+   BOOL res;
+   /* Try multiple times to send if packsize != nb since that probably
+@@ -751,8 +754,7 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
+       res = WriteFile (sendsig, leader, packsize, &nb, NULL);
+       if (!res || packsize == nb)
+ 	break;
+-      if (cygwait (NULL, 10, cw_sig_eintr) == WAIT_SIGNALED)
+-	_my_tls.call_signal_handler ();
++      cygwait (NULL, 10, cw_mask);
+       res = 0;
+     }
+
+@@ -785,7 +787,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);
++      rc = cygwait (pack.wakeup, WSSC, cw_mask);
+       ForceCloseHandle (pack.wakeup);
+     }
+   else
+@@ -806,9 +808,6 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
+       rc = -1;
+     }
+
+-  if (wait_for_completion && si.si_signo != __SIGFLUSHFAST)
+-    _my_tls.call_signal_handler ();
+-
+ out:
+   if (communing && rc)
+     {
+-- 
+2.47.1.windows.2
+
diff --git a/msys2-runtime/1004-Cygwin-signal-Avoid-frequent-TLS-lock-unlock-for-SIG.patch b/msys2-runtime/1004-Cygwin-signal-Avoid-frequent-TLS-lock-unlock-for-SIG.patch
@@ -0,0 +1,105 @@
+From 2be4821075cc6250b5c2d70f8a445925959e361b Mon Sep 17 00:00:00 2001
+From: Takashi Yano <[email protected]>
+Date: Mon, 20 Jan 2025 23:23:05 +0900
+Subject: [PATCH 1004/1004] Cygwin: signal: Avoid frequent TLS lock/unlock for
+ SIGCONT processing
+
+It seems that current _cygtls::handle_SIGCONT() code sometimes falls
+into a deadlock due to frequent TLS lock/unlock operation in the
+yield() loop. With this patch, the yield() in the wait loop is placed
+outside the TLS lock to avoid frequent TLS lock/unlock.
+
+Fixes: 9ae51bcc51a7 ("Cygwin: signal: Fix another deadlock between main and sig thread")
+Reviewed-by:
+Signed-off-by: Takashi Yano <[email protected]>
+---
+ winsup/cygwin/exceptions.cc           | 36 ++++++++++-----------------
+ winsup/cygwin/local_includes/cygtls.h |  4 +--
+ 2 files changed, 15 insertions(+), 25 deletions(-)
+
+diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
+index 469052a98a..000cc3daaf 100644
+--- a/winsup/cygwin/exceptions.cc
++++ b/winsup/cygwin/exceptions.cc
+@@ -1420,7 +1420,7 @@ api_fatal_debug ()
+
+ /* Attempt to carefully handle SIGCONT when we are stopped. */
+ void
+-_cygtls::handle_SIGCONT (threadlist_t * &tl_entry)
++_cygtls::handle_SIGCONT ()
+ {
+   if (NOTSTATE (myself, PID_STOPPED))
+     return;
+@@ -1431,23 +1431,17 @@ _cygtls::handle_SIGCONT (threadlist_t * &tl_entry)
+      Make sure that any pending signal is handled before trying to
+      send a new one.  Then make sure that SIGCONT has been recognized
+      before exiting the loop.  */
+-  bool sigsent = false;
+-  while (1)
+-    if (sig)		/* Assume that it's ok to just test sig outside of a
+-			   lock since setup_handler does it this way.  */
+-      {
+-	cygheap->unlock_tls (tl_entry);
+-	yield ();	/* Attempt to schedule another thread.  */
+-	tl_entry = cygheap->find_tls (_main_tls);
+-      }
+-    else if (sigsent)
+-      break;		/* SIGCONT has been recognized by other thread */
+-    else
+-      {
+-	sig = SIGCONT;
+-	set_signal_arrived (); /* alert sig_handle_tty_stop */
+-	sigsent = true;
+-      }
++  while (sig)  /* Assume that it's ok to just test sig outside of a */
++    yield ();          /* lock since setup_handler does it this way.  */
++
++  lock ();
++  sig = SIGCONT;
++  set_signal_arrived (); /* alert sig_handle_tty_stop */
++  unlock ();
++
++  while (sig == SIGCONT)
++    yield ();
++
+   /* Clear pending stop signals */
+   sig_clear (SIGSTOP, false);
+   sig_clear (SIGTSTP, false);
+@@ -1479,11 +1473,7 @@ sigpacket::process ()
+   myself->rusage_self.ru_nsignals++;
+
+   if (si.si_signo == SIGCONT)
+-    {
+-      tl_entry = cygheap->find_tls (_main_tls);
+-      _main_tls->handle_SIGCONT (tl_entry);
+-      cygheap->unlock_tls (tl_entry);
+-    }
++    _main_tls->handle_SIGCONT ();
+
+   /* SIGKILL is special.  It always goes through.  */
+   if (si.si_signo == SIGKILL)
+diff --git a/winsup/cygwin/local_includes/cygtls.h b/winsup/cygwin/local_includes/cygtls.h
+index 4bd79c36d7..328912f1ec 100644
+--- a/winsup/cygwin/local_includes/cygtls.h
++++ b/winsup/cygwin/local_includes/cygtls.h
+@@ -195,7 +195,7 @@ public: /* Do NOT remove this public: line, it's a marker for gentls_offsets. */
+   class cygthread *_ctinfo;
+   class san *andreas;
+   waitq wq;
+-  int sig;
++  volatile int sig;
+   unsigned incyg;
+   volatile unsigned spinning;
+   volatile unsigned stacklock;
+@@ -276,7 +276,7 @@ public: /* Do NOT remove this public: line, it's a marker for gentls_offsets. */
+   {
+     will_wait_for_signal = false;
+   }
+-  void handle_SIGCONT (threadlist_t * &);
++  void handle_SIGCONT ();
+   static void cleanup_early(struct _reent *);
+ private:
+   void call2 (DWORD (*) (void *, void *), void *, void *);
+-- 
+2.47.1.windows.2
+