forked from msys2/MSYS2-packages
-
Notifications
You must be signed in to change notification settings - Fork 32
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
msys2-runtime: update to 6469fc6ad6b20f6a1e7e84e1a4924f4bd77bf48d
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
- Loading branch information
Showing
3 changed files
with
84 additions
and
6 deletions.
There are no files selected for viewing
75 changes: 75 additions & 0 deletions
75
msys2-runtime/0054-cygthread-suspend-thread-before-terminating.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| From 6469fc6ad6b20f6a1e7e84e1a4924f4bd77bf48d Mon Sep 17 00:00:00 2001 | ||
| From: Johannes Schindelin <johannes.schindelin@gmx.de> | ||
| Date: Tue, 12 Nov 2024 22:21:08 +0100 | ||
| Subject: [PATCH 54/N] cygthread: suspend thread before terminating. | ||
|
|
||
| This addresses an extremely difficult to debug deadlock when running | ||
| under emulation on ARM64. | ||
|
|
||
| A relatively easy way to trigger this bug is to call `fork()`, then | ||
| within the child process immediately call another `fork()` and then | ||
| `exit()` the intermediate process. | ||
|
|
||
| It would seem that there is a "code emulation" lock on the wait thread | ||
| at this stage, and if the thread is terminated too early, that lock | ||
| still exists albeit without a thread, and nothing moves forward. | ||
|
|
||
| It seems that a `SuspendThread()` combined with a `GetThreadContext()` | ||
| (to force the thread to _actually_ be suspended, for more details see | ||
| https://devblogs.microsoft.com/oldnewthing/20150205-00/?p=44743) makes | ||
| sure the thread is "booted" from emulation before it is suspended. | ||
|
|
||
| Hopefully this means it won't be holding any locks or otherwise leave | ||
| emulation in a bad state when the thread is terminated. | ||
|
|
||
| Also, attempt to use `CancelSynchonousIo()` (as seen in `flock.cc`) to | ||
| avoid the need for `TerminateThread()` altogether. This doesn't always | ||
| work, however, so was not a complete fix for the deadlock issue. | ||
|
|
||
| Addresses: https://cygwin.com/pipermail/cygwin-developers/2024-May/012694.html | ||
| Signed-off-by: Jeremy Drake <cygwin@jdrake.com> | ||
| Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> | ||
| --- | ||
| winsup/cygwin/cygthread.cc | 14 ++++++++++++++ | ||
| winsup/cygwin/sigproc.cc | 3 ++- | ||
| 2 files changed, 16 insertions(+), 1 deletion(-) | ||
|
|
||
| diff --git a/winsup/cygwin/cygthread.cc b/winsup/cygwin/cygthread.cc | ||
| index 54918e7..4f16097 100644 | ||
| --- a/winsup/cygwin/cygthread.cc | ||
| +++ b/winsup/cygwin/cygthread.cc | ||
| @@ -302,6 +302,20 @@ cygthread::terminate_thread () | ||
| if (!inuse) | ||
| goto force_notterminated; | ||
|
|
||
| + if (_my_tls._ctinfo != this) | ||
| + { | ||
| + CONTEXT context; | ||
| + context.ContextFlags = CONTEXT_CONTROL; | ||
| + /* SuspendThread makes sure a thread is "booted" from emulation before | ||
| + it is suspended. As such, the emulator hopefully won't be in a bad | ||
| + state (aka, holding any locks) when the thread is terminated. */ | ||
| + SuspendThread (h); | ||
| + /* We need to call GetThreadContext, even though we don't care about the | ||
| + context, because SuspendThread is asynchronous and GetThreadContext | ||
| + will make sure the thread is *really* suspended before returning */ | ||
| + GetThreadContext (h, &context); | ||
| + } | ||
| + | ||
| TerminateThread (h, 0); | ||
| WaitForSingleObject (h, INFINITE); | ||
| CloseHandle (h); | ||
| diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc | ||
| index 5c5ff73..d180277 100644 | ||
| --- a/winsup/cygwin/sigproc.cc | ||
| +++ b/winsup/cygwin/sigproc.cc | ||
| @@ -410,7 +410,8 @@ proc_terminate () | ||
| if (!have_execed || !have_execed_cygwin) | ||
| chld_procs[i]->ppid = 1; | ||
| if (chld_procs[i].wait_thread) | ||
| - chld_procs[i].wait_thread->terminate_thread (); | ||
| + if (!CancelSynchronousIo (chld_procs[i].wait_thread->thread_handle ())) | ||
| + 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 | ||
| reach here when the next process has finished initializing but we |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 6183b8336e29b058b63dcf97ff8214b0355f165a | ||
| 6469fc6ad6b20f6a1e7e84e1a4924f4bd77bf48d |