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 25604e5
Showing
4 changed files
with
275 additions
and
3 deletions.
There are no files selected for viewing
75 changes: 75 additions & 0 deletions
75
msys2-runtime/1001-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,75 @@ | ||
| From 5c7cfd00d3588b3b5bf2befbc707bdb10b3b7eb3 Mon Sep 17 00:00:00 2001 | ||
| From: Jeremy Drake <[email protected]> | ||
| Date: Sat, 18 Jan 2025 12:23:54 -0800 | ||
| Subject: [PATCH 1/3] Revert "Cygwin: signal: Do not handle signal when | ||
| __SIGFLUSHFAST is sent" | ||
|
|
||
| This reverts commit 33799c250f0a7d353cfc34961c7bba26d3a4219a. | ||
| --- | ||
| 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 | ||
|
|
105 changes: 105 additions & 0 deletions
105
msys2-runtime/1002-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,105 @@ | ||
| From 3506dbadcf91161ec1e4eaa686debeab5fe617bb Mon Sep 17 00:00:00 2001 | ||
| From: Takashi Yano <[email protected]> | ||
| Date: Sat, 18 Jan 2025 19:03:23 +0900 | ||
| Subject: [PATCH 2/3] 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 for an | ||
| existing signal is placed outside the TLS lock. Conversely, the | ||
| yield() in the wait loop for a self-issued SIGCONT is placed inside | ||
| the TLS lock and do not unlock every time yield() is called. | ||
|
|
||
| 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 | 38 +++++++++++---------------- | ||
| winsup/cygwin/local_includes/cygtls.h | 2 +- | ||
| 2 files changed, 16 insertions(+), 24 deletions(-) | ||
|
|
||
| diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc | ||
| index 469052a98a..460d4ac89d 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,28 +1431,24 @@ _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 */ | ||
| + | ||
| + while (sig == SIGCONT) | ||
| + yield (); | ||
| + | ||
| /* Clear pending stop signals */ | ||
| sig_clear (SIGSTOP, false); | ||
| sig_clear (SIGTSTP, false); | ||
| sig_clear (SIGTTIN, false); | ||
| sig_clear (SIGTTOU, false); | ||
| + | ||
| + cygheap->unlock_tls (tl_entry); | ||
| } | ||
|
|
||
| int | ||
| @@ -1479,11 +1475,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 | ||
|
|
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 0f82cd14af4fd4191ce07f68aa6248a828492769 Mon Sep 17 00:00:00 2001 | ||
| From: Takashi Yano <[email protected]> | ||
| Date: Sat, 18 Jan 2025 19:21:51 +0900 | ||
| Subject: [PATCH 3/3] 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