Skip to content

Commit

Permalink
Cygwin: revert use of CancelSyncronousIo on wait_thread.
Browse files Browse the repository at this point in the history
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: msys2/MSYS2-packages#4340 (comment)
Fixes: b091b47 ("cygthread: suspend thread before terminating.")
Signed-off-by: Jeremy Drake <cygwin@jdrake.com>
  • Loading branch information
jeremyd2019 authored and lazka committed Nov 23, 2024
1 parent bdd7c8d commit fbc1019
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 17 deletions.
10 changes: 3 additions & 7 deletions winsup/cygwin/pinfo.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1210,17 +1210,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;
}

Expand Down
12 changes: 2 additions & 10 deletions winsup/cygwin/sigproc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -416,11 +416,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
Expand Down Expand Up @@ -1212,11 +1208,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 ())
Expand Down

0 comments on commit fbc1019

Please sign in to comment.