Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport patches for hangs #253

Merged
merged 4 commits into from
Jan 22, 2025

Conversation

jeremyd2019
Copy link
Member

@jeremyd2019 jeremyd2019 commented Jan 18, 2025

@jeremyd2019 jeremyd2019 force-pushed the msys2-3.5.5-takashi-patches branch from c5422e5 to 0f82cd1 Compare January 18, 2025 20:24
jeremyd2019 added a commit to jeremyd2019/MSYS2-packages that referenced this pull request Jan 18, 2025
As these are for testing, apply them separately and leave the
msys2-runtime PR as a draft for now
(msys2/msys2-runtime#253).
@jeremyd2019 jeremyd2019 force-pushed the msys2-3.5.5-takashi-patches branch 2 times, most recently from 746c3d7 to aa2ce2a Compare January 18, 2025 23:37
jeremyd2019 added a commit to jeremyd2019/MSYS2-packages that referenced this pull request Jan 18, 2025
As these are for testing, apply them separately and leave the
msys2-runtime PR as a draft for now
(msys2/msys2-runtime#253).
@jeremyd2019 jeremyd2019 force-pushed the msys2-3.5.5-takashi-patches branch from aa2ce2a to b6a94e2 Compare January 20, 2025 18:42
jeremyd2019 added a commit to jeremyd2019/MSYS2-packages that referenced this pull request Jan 20, 2025
As these are for testing, apply them separately and leave the
msys2-runtime PR as a draft for now
(msys2/msys2-runtime#253).
…ent"

This reverts commit a22a0ad to apply a new patch for the same
purpose.

Signed-off-by: Takashi Yano <[email protected]>
@jeremyd2019 jeremyd2019 force-pushed the msys2-3.5.5-takashi-patches branch from b6a94e2 to 365766a Compare January 21, 2025 05:59
jeremyd2019 added a commit to jeremyd2019/MSYS2-packages that referenced this pull request Jan 21, 2025
As these are for testing, apply them separately and leave the
msys2-runtime PR as a draft for now
(msys2/msys2-runtime#253).
tyan0 added 2 commits January 21, 2025 10:12
To allow cygwait() to be called in the signal handler, a locally
created timer is used instead of _cygtls::locals.cw_timer if it is
in use.

Co-Authored-By: Corinna Vinschen <[email protected]>
Signed-off-by: Takashi Yano <[email protected]>
(cherry picked from commit ea19140)
The commit a22a0ad was not entirely correct. Even with the patch,
some hangs still occur. This patch overrides the previous commit along
with the patch that makes cygwait() reentrant, to fix these hangs.

Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256954.html
Fixes: d243e51 ("Cygwin: signal: Fix deadlock between main thread and sig thread")
Reported-by: Daisuke Fujimura <[email protected]>
Reviewed-by: Corinna Vinschen <[email protected]>
Signed-off-by: Takashi Yano <[email protected]>
(cherry picked from commit 83afe3e)
@jeremyd2019 jeremyd2019 force-pushed the msys2-3.5.5-takashi-patches branch from 365766a to adb9c01 Compare January 21, 2025 18:12
jeremyd2019 added a commit to jeremyd2019/MSYS2-packages that referenced this pull request Jan 21, 2025
As these are for testing, apply them separately and leave the
msys2-runtime PR as a draft for now
(msys2/msys2-runtime#253).
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
outside the TLS lock to avoid frequent TLS lock/unlock.

Fixes: 9ae51bc ("Cygwin: signal: Fix another deadlock between main and sig thread")
Reviewed-by: Corinna Vinschen <[email protected]>
Signed-off-by: Takashi Yano <[email protected]>
@jeremyd2019 jeremyd2019 force-pushed the msys2-3.5.5-takashi-patches branch from 2be4821 to ef4923e Compare January 22, 2025 18:06
@jeremyd2019 jeremyd2019 changed the title test patches for hang backport patches for hangs Jan 22, 2025
@jeremyd2019
Copy link
Member Author

These patches are all upstream now, whether we want to merge this to backport them to 3.5.5 or wait for 3.5.6 is an open question.

@jeremyd2019 jeremyd2019 marked this pull request as ready for review January 22, 2025 18:26
@lazka
Copy link
Member

lazka commented Jan 22, 2025

Thanks. Since you already did the work, why not :)

@lazka lazka merged commit 25a3862 into msys2:msys2-3.5.5 Jan 22, 2025
1 check passed
@lazka
Copy link
Member

lazka commented Jan 22, 2025

I've created msys2/MINGW-packages#23167 to maybe trigger the hangs, and verify that the patches help. But not sure how easy/likely that is though.

@jeremyd2019
Copy link
Member Author

Keep in mind these will all (including the prior attempted hang fix) need to be dropped in the rebase for 3.5.6 when that happens

@jeremyd2019 jeremyd2019 deleted the msys2-3.5.5-takashi-patches branch January 22, 2025 21:50
@lazka
Copy link
Member

lazka commented Jan 22, 2025

All jobs in msys2/MINGW-packages#23167 seem to hang now, so that's a good test at least :)

@dscho
Copy link
Collaborator

dscho commented Jan 23, 2025

Just got word that v3.5.6 is mere days out 🎉

lazka pushed a commit to msys2/MSYS2-packages that referenced this pull request Jan 23, 2025
@dscho
Copy link
Collaborator

dscho commented Jan 24, 2025

FWIW (and this is in large part thanks to your encouragement, @jeremyd2019) I just merged git-for-windows/msys2-runtime#83 (which runs Git's test suite as part of git-for-windows/msys2-runtime's CI) and plan on exercising it immediately when rebasing to v3.5.6 (read: I want to open msys2/msys2-runtime and git-for-windows/msys2-runtime PRs at virtually the same time). That should help us gain more confidence when doing these risky Cygwin integrations in the future.

@dscho
Copy link
Collaborator

dscho commented Jan 26, 2025

Just got word that v3.5.6 is mere days out 🎉

It's out now.

@dscho
Copy link
Collaborator

dscho commented Jan 26, 2025

Just got word that v3.5.6 is mere days out 🎉

It's out now.

Draft PR is up (as well as the Git for Windows one so that we can catch regressions before merging).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants