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 20, 2025 · 8257f37 · 8257f37
1 parent f9fe3ae
commit 8257f37
Show file tree

Hide file tree

Showing 4 changed files with 337 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 8b6f3164ede6356d99152fcaa1abb3d0a903fc9f Mon Sep 17 00:00:00 2001
+From: Takashi Yano <[email protected]>
+Date: Tue, 21 Jan 2025 02:47:55 +0900
+Subject: [PATCH 1001/1003] 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,172 @@
+From 96aae8a0118e44160a8503974de17677816545e0 Mon Sep 17 00:00:00 2001
+From: Takashi Yano <[email protected]>
+Date: Tue, 21 Jan 2025 02:47:56 +0900
+Subject: [PATCH 1002/1003] 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]>
+---
+ winsup/cygwin/cygtls.cc               |  2 ++
+ winsup/cygwin/cygwait.cc              | 23 ++++++++++++++++-------
+ winsup/cygwin/local_includes/cygtls.h |  3 ++-
+ winsup/cygwin/select.cc               | 21 +++++++++++++++------
+ 4 files changed, 35 insertions(+), 14 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..4641a8417d 100644
+--- a/winsup/cygwin/cygwait.cc
++++ b/winsup/cygwin/cygwait.cc
+@@ -58,16 +58,21 @@ cygwait (HANDLE object, PLARGE_INTEGER timeout, unsigned mask)
+     }
+
+   DWORD timeout_n;
++  HANDLE *wait_timer = &_my_tls.locals.cw_timer;
++  HANDLE local_wait_timer = NULL;
+   if (!timeout)
+     timeout_n = WAIT_TIMEOUT + 1;
+   else
+     {
++      if (_my_tls.locals.cw_timer_inuse)
++	wait_timer = &local_wait_timer;
++      else
++	_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 +105,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 +113,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_wait_timer)
++	NtClose(local_wait_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..098b11f6b9 100644
+--- a/winsup/cygwin/select.cc
++++ b/winsup/cygwin/select.cc
+@@ -385,13 +385,18 @@ 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 *wait_timer = &_my_tls.locals.cw_timer;
++  HANDLE local_wait_timer = NULL;
+   if (us > 0LL)
+     {
+       NTSTATUS status;
+-      if (!wait_timer)
++      if (_my_tls.locals.cw_timer_inuse)
++	wait_timer = &local_wait_timer;
++      else
++	_my_tls.locals.cw_timer_inuse = true;
++      if (!*wait_timer)
+ 	{
+-	  status = NtCreateTimer (&wait_timer, TIMER_ALL_ACCESS, NULL,
++	  status = NtCreateTimer (wait_timer, TIMER_ALL_ACCESS, NULL,
+ 				  NotificationTimer);
+ 	  if (!NT_SUCCESS (status))
+ 	    {
+@@ -400,7 +405,7 @@ next_while:;
+ 	    }
+ 	}
+       LARGE_INTEGER ms_clock_ticks = { .QuadPart = -us * 10 };
+-      status = NtSetTimer (wait_timer, &ms_clock_ticks, NULL, NULL, FALSE,
++      status = NtSetTimer (*wait_timer, &ms_clock_ticks, NULL, NULL, FALSE,
+ 			   0, NULL);
+       if (!NT_SUCCESS (status))
+ 	{
+@@ -408,7 +413,7 @@ next_while:;
+ 			 status, ms_clock_ticks.QuadPart);
+ 	  return select_error;
+ 	}
+-      w4[m] = wait_timer;
++      w4[m] = *wait_timer;
+       timer_idx = m++;
+     }
+
+@@ -430,7 +435,11 @@ next_while:;
+   if (timer_idx)
+     {
+       BOOLEAN current_state;
+-      NtCancelTimer (wait_timer, &current_state);
++      NtCancelTimer (*wait_timer, &current_state);
++      if (local_wait_timer)
++	NtClose (local_wait_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,72 @@
+From b6a94e2621e2088140f4ab4a4585b42948044852 Mon Sep 17 00:00:00 2001
+From: Takashi Yano <[email protected]>
+Date: Tue, 21 Jan 2025 02:47:57 +0900
+Subject: [PATCH 1003/1003] Cygwin: signal: Do not handle signal when
+ __SIGFLUSHFAST is sent
+
+The commit a22a0ad7c4f0 was not exactly the correct thing. Even with
+the patch, some hangs still happen. This patch overrides the previous
+commit togerther with the patch making cygwait() reentrant to fix these
+hangs.
+
+Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256954.html
+Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256987.html
+Fixes: d243e51ef1d3 ("Cygwin: signal: Fix deadlock between main thread and sig thread")
+Fixes: a22a0ad7c4f0 ("Cygwin: signal: Do not handle signal when __SIGFLUSHFAST is sent")
+Reported-by: Daisuke Fujimura <[email protected]>
+Reported-by: Jeremy Drake <[email protected]>
+Reviewed-by:
+Signed-off-by: Takashi Yano <[email protected]>
+---
+ winsup/cygwin/sigproc.cc | 14 ++++++++------
+ 1 file changed, 8 insertions(+), 6 deletions(-)
+
+diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
+index cf43aa9335..474abe1495 100644
+--- a/winsup/cygwin/sigproc.cc
++++ b/winsup/cygwin/sigproc.cc
+@@ -742,6 +742,12 @@ sig_send (_pinfo *p, siginfo_t& si, _cygtls *tls)
+       memcpy (p, si._si_commune._si_str, n); p += n;
+     }
+
++  unsigned cw_mask;
++  if (pack.si.si_signo == __SIGFLUSHFAST)
++    cw_mask = 0;
++  else
++    cw_mask = cw_sig_restart;
++
+   DWORD nb;
+   BOOL res;
+   /* Try multiple times to send if packsize != nb since that probably
+@@ -751,8 +757,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 +790,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 +811,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/PKGBUILD b/msys2-runtime/PKGBUILD
@@ -4,7 +4,7 @@
 pkgbase=msys2-runtime
 pkgname=('msys2-runtime' 'msys2-runtime-devel')
 pkgver=3.5.5
-pkgrel=2
+pkgrel=3
 pkgdesc="Cygwin POSIX emulation engine"
 arch=('x86_64')
 url="https://www.cygwin.com/"
@@ -73,7 +73,10 @@ source=('msys2-runtime'::git://sourceware.org/git/newlib-cygwin.git#tag=cygwin-$
         0042-Cygwin-revert-use-of-CancelSyncronousIo-on-wait_thre.patch
         0043-Cygwin-cache-IsWow64Process2-host-arch-in-wincap.patch
         0044-Cygwin-uname-add-host-machine-tag-to-sysname.patch
-        0045-Cygwin-signal-Do-not-handle-signal-when-__SIGFLUSHFA.patch)
+        0045-Cygwin-signal-Do-not-handle-signal-when-__SIGFLUSHFA.patch
+        1001-Revert-Cygwin-signal-Do-not-handle-signal-when-__SIG.patch
+        1002-Cygwin-cygwait-Make-cygwait-reentrant.patch
+        1003-Cygwin-signal-Do-not-handle-signal-when-__SIGFLUSHFA.patch)
 sha256sums=('0c6fcf91be78369e753deb1963b4a04e0db613194e70a7ec653774b028adf509'
             '76e37d572d2aba473aab8f5a1984af2084e6069f9195795c71bc45778edbd1eb'
             '5c79b09f9337cc8a5f993db6dd1f54df269f8390ab3348a94e5a139a5d060e39'
@@ -119,7 +122,10 @@ sha256sums=('0c6fcf91be78369e753deb1963b4a04e0db613194e70a7ec653774b028adf509'
             '29c3412a1c7b0e4e719b64337ba5508b141037884ba96e9bee5f8ea253811aa3'
             '7064362256cb558fe443469b5f9988ca927667b7cf13f1c1020aca98e616f900'
             'fb6c38381eb4f36338a5184f79c98e6046c9ff741da37f2f38f4757c8714ca92'
-            '84bbe9320fe9fdc8bf6af88c9ae68eaff4bc8e162ba58ba7422bbd053e88a986')
+            '84bbe9320fe9fdc8bf6af88c9ae68eaff4bc8e162ba58ba7422bbd053e88a986'
+            '824472da2351262fcbabdd68414ebb85de7d9c1c8127a402ceb83a43b38e41e7'
+            'af3fde19a54572100b6ac03e8a1524a4c9188eb7351e7766005307a0e3107a77'
+            '8de0f419659eb78fa749328784a1375a96059e2f4696aa0d072013d4fa59358b')
 
 # Helper macros to help make tasks easier #
 apply_patch_with_msg() {
@@ -202,6 +208,12 @@ prepare() {
   0043-Cygwin-cache-IsWow64Process2-host-arch-in-wincap.patch \
   0044-Cygwin-uname-add-host-machine-tag-to-sysname.patch \
   0045-Cygwin-signal-Do-not-handle-signal-when-__SIGFLUSHFA.patch
+
+ # test patches for @tyan0
+ apply_patch_with_msg \
+   1001-Revert-Cygwin-signal-Do-not-handle-signal-when-__SIG.patch \
+   1002-Cygwin-cygwait-Make-cygwait-reentrant.patch \
+   1003-Cygwin-signal-Do-not-handle-signal-when-__SIGFLUSHFA.patch
 }
 
 build() {