Backport fixes for reported hangs - #144
Merged
Merged
Conversation
dscho
force-pushed
the
gfw-console-mode-and-owner-exit
branch
from
September 25, 2026 14:49
8a9a57a to
3c7d3b1
Compare
dscho
changed the title
dscho
marked this pull request as ready for review
September 25, 2026 16:14
This comment was marked as outdated.
This comment was marked as outdated.
Since the commit 31bf91f867c5, opening fifo causes a deadlock. This is because, open_with_arch() for fifo can be blocked until the other side of the fifo is opened. The commit 31bf91f867c5 moves the creating cygheap_fdnew before open_with_arch() to address the issue: https://cygwin.com/pipermail/cygwin/2026-May/259664.html However, cygheap_fdnew locks fdtab, so open() for the other side of fifo cannot create cygheap_fdnew until fdtab is unlocked. This is the cause of the deadlock. With this patch, fdtab is unlocked before open_with_arch(), but marked as used using tentative fhandler. The summary of open() is as follows. 1) Lock fdtab. 2) Create new fd. 3) Mark fd as used using tentative fhandler. 4) Unlock fdtab. 5) Call open_with_arch(). 6) Set final fhandler to fd. The important point is that create fd before open_with_arch() to address https://cygwin.com/pipermail/cygwin/2026-May/259664.html, but unlock fdtab before open_with_arch() to address https://cygwin.com/pipermail/cygwin/2026-July/259884.html. Fixes: 31bf91f867c5 ("Cygwin: Ensure unused fd available for open()") Addresses: https://cygwin.com/pipermail/cygwin/2026-July/259884.html Reported-by: kikairoya <kikairoya@gmail.com> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Mark Geisert <mark@maxrnd.com> (cherry picked from commit 524d75ff73986b263161665af771cc90e55b5e01)
The commit fac73911f5a0 ("Cygwin: console: Fix typeahead input for
bash") introduced a bug where select() consumes some input chars in
canonical mode, preventing read() from reading them. This is due to
discarding input events when process_input_message() does not return
`input_ok` even if it is called from select().
The basic idea of that commit was making process_input_message()
not to store processed chars into readahead buffer. This was not
correct because the key input events were processed twice, once by
select() and again by read(). Thus even if that commit worked as
intended, the side effect such as input echo would be applied twice.
With this patch, process_input_message() handles only the minimum
necessary of input events in both cases, those processed by select()
and those processed by read(). To achieve this behaviour, the function
returns without processing when `input_ready` is already satisfied,
or after it has processed the specified number of chars.
Addresses: https://cygwin.com/pipermail/cygwin/2026-August/259915.html
Reported-by: Steven Doerfler <sgd-cygwinlist@lugaru.com>
Fixes: fac73911f5a0 ("Cygwin: console: Fix typeahead input for bash")
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Revewied-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
(cherry picked from commit 9807479c90e37913900569d3a8f3b10247bf7860)
Currently, when cygwin app is launched, the console input mode is
set to tty::cygwin, even if the stdin is not a console. However,
it is not necessary because the cygwin app does not use stdin.
This also applies to stdout and stderr.
With this patch, the console mode is set only when std{in,out,err}
is a console for the cygwin app for better coexistence with non-
cygwin apps.
This is a prerequisite for the experimental backport of Takashi Yano's
v15 console-mode patch to msys2-3.6.10. The release branch's later,
already-applied suspension-before-mode ordering is preserved.
(cherry picked from commit bbd3710fc83451426e3a58e5032437ca535fa444)
Assisted-by: GPT-6
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Previously, if two non-cygwin apps are started and one of them exits first, the other one lost appropriate console mode, since the first one restored it to tty::cygwin. This patch counts the active console process whose pgid is pgid of the tty and if the result is zero (means the last non-cygwin foreground process), restore console mode. To avoid race issue between apps modifying console mode simultaneously, this patch also introduce a mutex named `cons_mode_mutex`. Known limitation: In the case of non-overlayed spawn, there still exists a small window in which another non-cygwin process may restore tty::cygwin mode even though new non-cygwin app is about to start. In addition: Avoid the cleanup/startup deadlock: cleanup can hold `cons_mode_mutex` while waiting for a console master that a new owner cannot start until acquiring that mutex. Startup must precede the mode-mutex wait without changing the handshake or mode serialization. Backport the complete upstream v18 patch after the prerequisite: https://inbox.sourceware.org/cygwin-patches/20260919004709.26360-1-takashi.yano@nifty.ne.jp/ Adapt only the three existing msys2-3.6.10 preimage/context spellings. Fixes: 48285aa ("Cygwin: console: Fix handling of Ctrl-S in Win7.") Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Assisted-by: GPT-6 Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
With the commit 733d5a953fa9 ("Cygwin: console: Ensure the master
thread runs only when it is supposed to"), the process which calls
set_disable_master_thread() hangs if the con.owner already exited,
because set_disable_master_thread() waits for cons_master_thread
accepting the status change and reflecting the current status to
master_thread_suspended. With this patch, set_disable_master_thread()
is aborted if the owner process no longer exists to avoid this
hang.
Backport the complete upstream v5 patch unchanged after v18:
https://inbox.sourceware.org/cygwin-patches/20260923223230.3504-1-takashi.yano@nifty.ne.jp/
Addresses: https://cygwin.com/pipermail/cygwin/2026-September/260037.html
Fixes: 733d5a953fa9 ("Cygwin: console: Ensure the master thread runs only when it is supposed to")
Reported-by: Jay Libove Alzina <libove@felines.org>
Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Assisted-by: GPT-6
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Bring the MSYS2 console-mode and owner-exit changes into Git for Windows without losing its pcon-backed PTY handling. For Cygwin grandchildren of native processes, close and invalidate the inherited pseudo-console handle only when a pcon-backed PTY is found; real console handles must remain open for console-mode setup. The incoming history also includes an fd-table locking fix and the console-input regression fix proposed in Git for Windows PR #141. Assisted-by: GPT-6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The 3.6.10-4 runtime hangs under concurrent jobs in a native Windows console. Cover this in CI with a real console: redirected stdio would bypass the affected path. This passes successfully with the current revision, but would fail without the fix merged via the parent commit. Assisted-by: GPT-6 Sol Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The 99-job console race test needs native children with variable lifetimes to exercise console handoffs. Currently, it uses `ping` invocation that stay alive for a randomized duration. However, `ping` uses the network (even if it is only a loopback device it pings), which adds a surface for flakiness that I do not want, and which I missed when Sol thought it a good idea to ignore my instruction to merely sleep instead. Assisted-by: GPT-6 Sol Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Ctrl+C must still interrupt the Cygwin side of a pipeline while the native side owns console input. Cover this separately from the 99-job console race. This was discussed in: https://inbox.sourceware.org/cygwin-patches/20260923105326.d57b0710f1519e32b9c5d496@nifty.ne.jp/ Assisted-by: GPT-6 Sol Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
force-pushed
the
gfw-console-mode-and-owner-exit
branch
from
September 26, 2026 15:40
3c7d3b1 to
affbfc5
Compare
Member
Author
|
I know it's a bit late in the -rc cycle, but I am running afoul of these issues myself. I did my best (and the patch author much more so: these reflect v18 and v5!!! of the patches, respectively, he truly is one of the unsung heroes of open source to which https://xkcd.com/2347/ refers). So I am quite confident that the fixes are sound and should be part of Git for Windows v2.56.0. |
Member
Author
|
/open pr The workflow run was started |
Hondarer
added a commit
to Hondarer/devbin-win
that referenced
this pull request
Sep 28, 2026
The MSYS runtime 3.6.10 bundled with PortableGit 2.55.0.5 can hang when Ctrl-C is pressed during make: the native make.exe exits, but the MSYS exec stub sh.exe stays behind and the prompt never returns. The fix (git-for-windows/msys2-runtime#144, tracked in git-for-windows/git#6442) ships in v2.56.0.windows.1. - Download from GitHub releases, since the SourceForge mirror does not have 2.56.0 yet - Update the README link and the redistribution audit entry Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BGfgLy4TYhBeRzbJm3PVc1
dscho
added a commit
that referenced
this pull request
Oct 2, 2026
In the MSYS2 project, there were several reports of newly-introduced hangs, e.g. msys2/msys2-runtime#351 and msys2/MSYS2-packages#6445 (comment). It took a good while to figure out how to fix this properly, with two separate fixes needing 18 and 5 iterations, respectively, on the cygwin-patches mailing list. This branch merges [MSYS2 runtime PR #368](msys2/msys2-runtime#368). The sole `dtable.cc` conflict preserves GFW's pcon-backed PTY detection and genuine console handles. Separately, tests running in CI builds are added that replicate the reproducer of issue 351 and a separate Ctrl+C test that covers [Takashi's pipeline scenario](https://inbox.sourceware.org/cygwin-patches/20260923105326.d57b0710f1519e32b9c5d496@nifty.ne.jp/): the Cygwin sleeper must be interrupted via Ctrl+C even when the native side owns console input. The original DLL already passed this test, so it protects against regressions in this existing behavior.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the MSYS2 project, there were several reports of newly-introduced hangs, e.g. msys2/msys2-runtime#351 and msys2/MSYS2-packages#6445 (comment). It took a good while to figure out how to fix this properly, with two separate fixes needing 18 and 5 iterations, respectively, on the cygwin-patches mailing list.
This branch merges MSYS2 runtime PR #368. The sole
dtable.ccconflict preserves GFW's pcon-backed PTY detection and genuine console handles.Separately, tests running in CI builds are added that replicate the reproducer of issue 351 and a separate Ctrl+C test that covers Takashi's pipeline scenario: the Cygwin sleeper must be interrupted via Ctrl+C even when the native side owns console input. The original DLL already passed this test, so it protects against regressions in this existing behavior.