forked from msys2/MSYS2-packages
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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).
- Loading branch information
1 parent
f9fe3ae
commit 11efa75
Showing
4 changed files
with
268 additions
and
3 deletions.
There are no files selected for viewing
96 changes: 96 additions & 0 deletions
96
msys2-runtime/1001-Cygwin-signal-Avoid-frequent-tls-lock-unlock-for-SIG.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,96 @@ | ||
| From 1c45d1d6b02cf18ad05c692e982309b0aa19999d Mon Sep 17 00:00:00 2001 | ||
| From: Takashi Yano <[email protected]> | ||
| Date: Sat, 18 Jan 2025 19:03:23 +0900 | ||
| Subject: [PATCH 1001/1003] 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 | ||
| 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 | 2 +- | ||
| 2 files changed, 14 insertions(+), 24 deletions(-) | ||
|
|
||
| diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc | ||
| index 469052a98a..18ea2ffb29 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) | ||
| + yield (); | ||
| + | ||
| + threadlist_t *tl_entry = cygheap->find_tls (this); | ||
| + sig = SIGCONT; | ||
| + set_signal_arrived (); /* alert sig_handle_tty_stop */ | ||
| + cygheap->unlock_tls (tl_entry); | ||
| + | ||
| + 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 e4e3889aff..7dea6de8a7 100644 | ||
| --- a/winsup/cygwin/local_includes/cygtls.h | ||
| +++ b/winsup/cygwin/local_includes/cygtls.h | ||
| @@ -275,7 +275,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 | ||
|
|
77 changes: 77 additions & 0 deletions
77
msys2-runtime/1002-Revert-Cygwin-signal-Do-not-handle-signal-when-__SIG.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,77 @@ | ||
| From 57ea7814c175163485fd376fb5a208723385f6e9 Mon Sep 17 00:00:00 2001 | ||
| From: Jeremy Drake <[email protected]> | ||
| Date: Sat, 18 Jan 2025 12:23:54 -0800 | ||
| Subject: [PATCH 1002/1003] Revert "Cygwin: signal: Do not handle signal when | ||
| __SIGFLUSHFAST is sent" | ||
|
|
||
| This reverts commit 33799c250f0a7d353cfc34961c7bba26d3a4219a. | ||
|
|
||
| Signed-off-by: Jeremy Drake <[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 | ||
|
|
80 changes: 80 additions & 0 deletions
80
msys2-runtime/1003-Cygwin-signal-Do-not-handle-signal-when-__SIGFLUSHFA.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,80 @@ | ||
| From aa2ce2a8a1db49dfe2ca880b372c9948da6bb1a6 Mon Sep 17 00:00:00 2001 | ||
| From: Takashi Yano <[email protected]> | ||
| Date: Sat, 18 Jan 2025 19:21:51 +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 to fix these hangs. | ||
|
|
||
| Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256987.html | ||
| Fixes: a22a0ad7c4f0 ("Cygwin: signal: Do not handle signal when __SIGFLUSHFAST is sent") | ||
| Reported-by: Jeremy Drake <[email protected]> | ||
| Reviewed-by: | ||
| Signed-off-by: Takashi Yano <[email protected]> | ||
| --- | ||
| winsup/cygwin/sigproc.cc | 33 +++++++++++++++++++++++++++------ | ||
| 1 file changed, 27 insertions(+), 6 deletions(-) | ||
|
|
||
| diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc | ||
| index cf43aa9335..20046eb738 100644 | ||
| --- a/winsup/cygwin/sigproc.cc | ||
| +++ b/winsup/cygwin/sigproc.cc | ||
| @@ -751,8 +751,19 @@ 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 (); | ||
| + if (pack.si.si_signo == __SIGFLUSHFAST) | ||
| + Sleep (10); | ||
| + else /* Handle signals */ | ||
| + do | ||
| + { | ||
| + DWORD rc = WaitForSingleObject (_my_tls.get_signal_arrived (), 10); | ||
| + if (rc == WAIT_OBJECT_0) | ||
| + { | ||
| + _my_tls.call_signal_handler (); | ||
| + continue; | ||
| + } | ||
| + } | ||
| + while (false); | ||
| res = 0; | ||
| } | ||
|
|
||
| @@ -785,7 +796,20 @@ 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); | ||
| + if (pack.si.si_signo == __SIGFLUSHFAST) | ||
| + rc = WaitForSingleObject (pack.wakeup, WSSC); | ||
| + else /* Handle signals */ | ||
| + do | ||
| + { | ||
| + HANDLE w[2] = {pack.wakeup, _my_tls.get_signal_arrived ()}; | ||
| + rc = WaitForMultipleObjects (2, w, FALSE, WSSC); | ||
| + if (rc == WAIT_OBJECT_0 + 1) /* signal arrived */ | ||
| + { | ||
| + _my_tls.call_signal_handler (); | ||
| + continue; | ||
| + } | ||
| + } | ||
| + while (false); | ||
| ForceCloseHandle (pack.wakeup); | ||
| } | ||
| else | ||
| @@ -806,9 +830,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 | ||
|
|
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