msys2-runtime: revert use of CancelSyncronousIo on wait_thread

This change seems to have caused hangs on x86_64, so let's revert it. Addresses #4340 (comment) and corresponds to msys2/msys2-runtime#243. Signed-off-by: Johannes Schindelin <[email protected]>
msys2 · Nov 23, 2024 · 2e69bb0 · 2e69bb0
1 parent 0e506b8
commit 2e69bb0
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 4 deletions.
diff --git a/msys2-runtime/0045-Cygwin-revert-use-of-CancelSyncronousIo-on-wait_thre.patch b/msys2-runtime/0045-Cygwin-revert-use-of-CancelSyncronousIo-on-wait_thre.patch
@@ -0,0 +1,73 @@
+From 2eb6be14ee7baeff294a7297260fb47b40f59679 Mon Sep 17 00:00:00 2001
+From: Jeremy Drake <[email protected]>
+Date: Thu, 21 Nov 2024 22:13:42 -0800
+Subject: [PATCH 45/N] Cygwin: revert use of CancelSyncronousIo on
+ wait_thread.
+
+It appears this is causing hangs on native x86_64 in similar scenarios
+as the hangs on ARM64, because `CancelSynchronousIo` is returning `TRUE`
+but not canceling the `ReadFile` call as expected.
+
+Addresses: https://github.com/msys2/MSYS2-packages/issues/4340#issuecomment-2491401847
+Fixes: b091b47b9e56 ("cygthread: suspend thread before terminating.")
+Signed-off-by: Jeremy Drake <[email protected]>
+---
+ winsup/cygwin/pinfo.cc   | 10 +++-------
+ winsup/cygwin/sigproc.cc | 12 ++----------
+ 2 files changed, 5 insertions(+), 17 deletions(-)
+
+diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
+index 4bb1946..a5f5d6e 100644
+--- a/winsup/cygwin/pinfo.cc
++++ b/winsup/cygwin/pinfo.cc
+@@ -1262,17 +1262,13 @@ proc_waiter (void *arg)
+
+   for (;;)
+     {
+-      DWORD nb, err;
++      DWORD nb;
+       char buf = '\0';
+
+       if (!ReadFile (vchild.rd_proc_pipe, &buf, 1, &nb, NULL)
+-	  && (err = GetLastError ()) != ERROR_BROKEN_PIPE)
++	  && GetLastError () != ERROR_BROKEN_PIPE)
+ 	{
+-	  /* ERROR_OPERATION_ABORTED is expected due to the possibility that
+-	     CancelSynchronousIo interruped the ReadFile call, so don't output
+-	     that error */
+-	  if (err != ERROR_OPERATION_ABORTED)
+-	    system_printf ("error on read of child wait pipe %p, %E", vchild.rd_proc_pipe);
++	  system_printf ("error on read of child wait pipe %p, %E", vchild.rd_proc_pipe);
+ 	  break;
+ 	}
+
+diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
+index 19a2aec..a89f09d 100644
+--- a/winsup/cygwin/sigproc.cc
++++ b/winsup/cygwin/sigproc.cc
+@@ -409,11 +409,7 @@ proc_terminate ()
+ 	     to 1 iff it is a Cygwin process.  */
+ 	  if (!have_execed || !have_execed_cygwin)
+ 	    chld_procs[i]->ppid = 1;
+-	  /* Attempt to exit the wait_thread cleanly via CancelSynchronousIo
+-	     before falling back to the (explicitly dangerous) cross-thread
+-	     termination */
+-	  if (chld_procs[i].wait_thread
+-	      && !CancelSynchronousIo (chld_procs[i].wait_thread->thread_handle ()))
++	  if (chld_procs[i].wait_thread)
+ 	    chld_procs[i].wait_thread->terminate_thread ();
+ 	  /* Release memory associated with this process unless it is 'myself'.
+ 	     'myself' is only in the chld_procs table when we've execed.  We
+@@ -1178,11 +1174,7 @@ remove_proc (int ci)
+ {
+   if (have_execed)
+     {
+-      /* Attempt to exit the wait_thread cleanly via CancelSynchronousIo
+-	 before falling back to the (explicitly dangerous) cross-thread
+-	 termination */
+-      if (_my_tls._ctinfo != chld_procs[ci].wait_thread
+-	  && !CancelSynchronousIo (chld_procs[ci].wait_thread->thread_handle ()))
++      if (_my_tls._ctinfo != chld_procs[ci].wait_thread)
+ 	chld_procs[ci].wait_thread->terminate_thread ();
+     }
+   else if (chld_procs[ci] && chld_procs[ci]->exists ())
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.4
-pkgrel=6
+pkgrel=7
 pkgdesc="Cygwin POSIX emulation engine"
 arch=('x86_64')
 url="https://www.cygwin.com/"
@@ -72,7 +72,8 @@ source=('msys2-runtime'::git://sourceware.org/git/newlib-cygwin.git#tag=cygwin-$
         0041-Cygwin-find_fast_cwd-don-t-run-assembler-checking-co.patch
         0042-cygthread-suspend-thread-before-terminating.patch
         0043-fixup-cygthread-suspend-thread-before-terminating.patch
-        0044-fixup-cygthread-suspend-thread-before-terminating.patch)
+        0044-fixup-cygthread-suspend-thread-before-terminating.patch
+        0045-Cygwin-revert-use-of-CancelSyncronousIo-on-wait_thre.patch)
 sha256sums=('b8dce32fd9746506752d90ac3f30454fe1689100b08c41442016aaf244cc8584'
             '9f9e1b6b05cbc9a715fe9443740b25171e9c1a276a058e6ba7e4f6eada6872c8'
             'e5b2095e543a5d702cfce6da26cd17a78f40e17620315b1bcc434b94a007ae9b'
@@ -117,7 +118,8 @@ sha256sums=('b8dce32fd9746506752d90ac3f30454fe1689100b08c41442016aaf244cc8584'
             '34035a411acb71c81a7f4a2367d2cf9f7f00572b6e92c7ba5506e6a48e4867ca'
             '6ae29efcd4d17aad01eed252d166de4dd13c0bb2274905933152a1eb21c517dc'
             '1c08c1c6ff588b8a3db23b8506c3e2c52c207f363d7c04b44da50640f176aab6'
-            'd40da853f11607c7c4bfe5abd95499a2042e520bb483b62fdae34182907f8d74')
+            'd40da853f11607c7c4bfe5abd95499a2042e520bb483b62fdae34182907f8d74'
+            'b580775232a40bffa60798564988ab2827d0b179db21988552a8533ec8e5098d')
 
 # Helper macros to help make tasks easier #
 apply_patch_with_msg() {
@@ -198,7 +200,8 @@ prepare() {
   0041-Cygwin-find_fast_cwd-don-t-run-assembler-checking-co.patch \
   0042-cygthread-suspend-thread-before-terminating.patch \
   0043-fixup-cygthread-suspend-thread-before-terminating.patch \
-  0044-fixup-cygthread-suspend-thread-before-terminating.patch
+  0044-fixup-cygthread-suspend-thread-before-terminating.patch \
+  0045-Cygwin-revert-use-of-CancelSyncronousIo-on-wait_thre.patch
 }
 
 build() {