diff --git a/msys2-runtime/0050-symlink_native-allow-linking-to.patch b/msys2-runtime/0050-symlink_native-allow-linking-to.patch new file mode 100644 index 00000000000..f8a922e17af --- /dev/null +++ b/msys2-runtime/0050-symlink_native-allow-linking-to.patch @@ -0,0 +1,62 @@ +From bc3e285ac0a2c91080ac68279f0c9fd697a2d9b6 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Thu, 19 Jun 2025 20:46:03 +0200 +Subject: [PATCH 50/N] symlink_native: allow linking to `..` + +When running + + CYGWIN=winsymlinks:nativestrict ln -s .. abc + +the counter-intuitive result is _not_ a symbolic link to `..`, but +instead to `../../$(basename "$PWD")`. + +The reason for this is that the search for the longest common prefix +assumes that the link target is not a strict prefix of the parent +directory of the link itself. + +Let's fix that. + +Signed-off-by: Johannes Schindelin +--- + winsup/cygwin/path.cc | 21 ++++++++++++++++----- + 1 file changed, 16 insertions(+), 5 deletions(-) + +diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc +index 6f04658..ead7d16 100644 +--- a/winsup/cygwin/path.cc ++++ b/winsup/cygwin/path.cc +@@ -2030,9 +2030,18 @@ symlink_native (const char *oldpath, path_conv &win32_newpath) + while (towupper (*++c_old) == towupper (*++c_new)) + ; + /* The last component could share a common prefix, so make sure we end +- up on the first char after the last common backslash. */ +- while (c_old[-1] != L'\\') +- --c_old, --c_new; ++ up on the first char after the last common backslash. ++ ++ However, if c_old is a strict prefix of c_new (at a component ++ boundary), or vice versa, then do not try to find the last common ++ backslash. */ ++ if ((!*c_old || *c_old == L'\\') && (!*c_new || *c_new == L'\\')) ++ c_old += !!*c_old, c_new += !!*c_new; ++ else ++ { ++ while (c_old[-1] != L'\\') ++ --c_old, --c_new; ++ } + + /* 2. Check if prefix is long enough. The prefix must at least points to + a complete device: \\?\X:\ or \\?\UNC\server\share\ are the minimum +@@ -2057,8 +2066,10 @@ symlink_native (const char *oldpath, path_conv &win32_newpath) + final_oldpath = &final_oldpath_buf; + final_oldpath->Buffer = tp.w_get (); + PWCHAR e_old = final_oldpath->Buffer; +- while (num-- > 0) +- e_old = wcpcpy (e_old, L"..\\"); ++ while (num > 1 || (num == 1 && *c_old)) ++ e_old = wcpcpy (e_old, L"..\\"), num--; ++ if (num > 0) ++ e_old = wcpcpy (e_old, L".."); + wcpcpy (e_old, c_old); + } + } diff --git a/msys2-runtime/0051-pipe-fix-SSH-hang-again.patch b/msys2-runtime/0051-pipe-fix-SSH-hang-again.patch new file mode 100644 index 00000000000..3aa88f766db --- /dev/null +++ b/msys2-runtime/0051-pipe-fix-SSH-hang-again.patch @@ -0,0 +1,206 @@ +From 95b4ba8a80a4d79f4d98401c7b22d16cffb1fca9 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Mon, 23 Jun 2025 13:06:02 +0200 +Subject: [PATCH 51/N] pipe: fix SSH hang (again) +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +When cbfaeba4f7 (Cygwin: pipe: Fix incorrect write length in +raw_write(), 2024-11-06) fixed a bug where too-long writes could cause +segmentation faults, it not only left out crucial details from the +commit message, it also introduced a bug. + +This manifests e.g. in the symptom where cloning large repositories in +Git for Windows via SSH "freeze" at random stages, as has been reported +in https://github.com/git-for-windows/git/issues/5688 and in +https://github.com/git-for-windows/git/issues/5682. + +The bug in question was to use `is_nonblocking ()` instead of +`real_non_blocking_mode` in a newly introduced condition. If the commit +message had filled in those details, it would most likely have been much +easier to spot the bug before the patch was committed. + +Granted, the patch _sort of_ moved this `is_nonblocking ()` condition +from another place, which means that the bug was present before, even if +it did not have the same detrimental symptom of hanging a clone of a +large repository via SSH. + +The _original_ bug was introduced in the equally under-explained +7ed9adb356 (Cygwin: pipe: Switch pipe mode to blocking mode by default, +2024-09-05). + +What is ironic is that this here patch is the latest (and hopefully +last) commit in a _long_ chain of bug fixes that fix bugs introduced by +preceding bug fixes: + +- 9e4d308cd5 (Cygwin: pipe: Adopt FILE_SYNCHRONOUS_IO_NONALERT flag for + read pipe., 2021-11-10) fixed a bug where Cygwin hung by mistake while + piping output from one .NET program as input to another .NET program + (potentially introduced by 365199090c (Cygwin: pipe: Avoid false EOF + while reading output of C# programs., 2021-11-07), which was itself a + bug fix). It introduced a bug that was fixed by... +- fc691d0246 (Cygwin: pipe: Make sure to set read pipe non-blocking for + cygwin apps., 2024-03-11). Which introduced a bug that was purportedly + fixed by... +- 7ed9adb356 (Cygwin: pipe: Switch pipe mode to blocking mode by + default, 2024-09-05). Which introduced a bug that was fixed by... +- cbfaeba4f7 (Cygwin: pipe: Fix incorrect write length in raw_write(), + 2024-11-06). Which introduced a bug that was fixed by... this here + patch. + +There is not only the common thread here that each of these bug fixes +introduced a new bug, but also the common thread of commit messages that +leave the reader puzzled and the problem — as well as the solution — +under-explained. It is quite likely that, had the time and care been +spent to fully explain what is going on in the commit messages, this +would quite likely have triggered a re-review of the diff by the author. +In other words, writing a complete commit message could have increased +the insight and understanding of the problem and thereby prevented this +series of "fixes that introduce new bugs". + +To avoid making the same mistake once again, here is my attempt at a +thorough explanation of what's going on, what is the problem, what +solutions were considered, and why the "winner" was chosen. + +So let's start with an overview of the logic of the +`fhandler_pipe_fifo::raw_write ()` method. + +This method implements the fundamental logic behind writing to any pipe +in Cygwin, whether it be blocking or not blocking, and takes as input +two variables, `ptr` and `len`, pointing to the data to be written. + +First, this method determines a couple of initial conditions (such as +maybe waiting on a specific mutex, or determining whether the pipe had +been closed already, or whether it is in blocking or in non-blocking +mode). Then, this method determines how many bytes of data should be +written in each attempt (assigned to the variable `chunk`). + +The amount of bytes already written is tracked as `nbytes`. While this +value is smaller than `len`, the central loop of this method is +executed. + +Counterintuitively, this loop is necessary even in non-blocking mode, to +handle conditions like `STATUS_PENDING`. + +Inside this loop body, a variety of conditions are handled, including +some that break out of that loop before `nbytes >= len`, e.g. when the +pipe is closed on the other end. + +This loop body also contains special, complicated logic to handle the +case where the pipe buffer is not large enough to handle the current +`chunk`, trying to write even less bytes (this is guarded by the flag +`short_write_once`). + +Unfortunately, the recent re-design of that logic decided to always set +the pipe to blocking (cf. the diff of ed9adb356 (Cygwin: pipe: Switch +pipe mode to blocking mode by default, 2024-09-05)). The rationale for +that decision was the desire to simplify the code, which came at the +expense of working well together with pipes that were created outside of +Cygwin and that are set to non-blocking mode. + +To this end, the current implementation now _emulates_ non-blocking +behavior while keeping the actual pipe in blocking mode. This requires +knowing how much data can be written to the pipe without blocking at any +given moment. Because by knowing that amount, even in blocking mode a +write operation can be performed that does not block. This information +is obtained via `NtQueryInformationFile(FilePipeLocalInformation)` and +stored in the variable `avail`. + +However, if for some reason it cannot be determined how much space is +available in the pipe, this non-blocking mode emulation would not work. +Therefore the pipe is set to genuine non-blocking mode in such cases, +and only in such cases. This minimizes the time window (but does not +eliminate it completely!) during which Cygwin does not work well +together with non-Cygwin processes via pipes. + +It also shows that the original desire to simplify the code can not be +fulfilled using the current strategy, as there is now both code to +emulate non-blocking mode as well as code that handles genuinely +non-blocking mode. + +This information whether non-blocking mode is merely emulated or +genuinely enabled, is tracked by the Boolean variable +`real_non_blocking_mode`. + +At the beginning of the loop body, after determining how many bytes to +write (stored in `len1`), the `NtWriteFile()` is called. Since the +re-design to always use blocking mode (at least insofar possible), this +`NtWriteFile()` call is guarded by a condition so that it is called only +once in non-blocking mode. + +Now, the logic for that is quite complex. First, `len1` is clamped to +`chunk` in blocking mode, otherwise to however many bytes are left to +write. Then, an _inner_ `while` loop runs as long as `len1` is greater +than 0. Inside that loop, `NtWriteFile()` is called _unless_ we'd try to +write more bytes than are available in emulated non-blocking mode. + +It is the complex interplay between that clamping to `chunk` and the +condition when `NtWriteFile()` is skipped that is at the heart of this +bug. + +But let's first see what happens in the case described above, where the +(Cygwin) SSH process gets stuck writing to the (non-Cygwin) Git process. +In the instances I observed, the big `while (nbytes < len)` loop is +started with the following initial conditions: `len` is 2097152, `chunk` +and `avail` are 1 and `real_non_blocking_mode` is 0. + +When the outer loop is entered, `len1` is therefore also set to 2097152, +and at the start of the inner loop the condition `len1 > avail` holds +true. Therefore, the `NtWriteFile()` call is skipped already during the +first iteration, we run into the code that tries a short write (setting +`short_write_once` to 1), and ultimately fail and return from +`raw_write()` with an error. + +Since not a single byte was written, the caller will try again at some +stage, failing again, over and over, and we're in an infinite loop. + +The fix chosen in this here commit is to apply that clamping to `chunk` +_also_ in the emulated non-blocking mode. This lets the `NtWriteFile()` +call _not_ be skipped in non-emulated non-blocking mode, as was clearly +the intention. + +An alternative patch that would have also addressed the symptom would be +to partially revert this hunk of cbfaeba4f7 (Cygwin: pipe: Fix incorrect +write length in raw_write(), 2024-11-06): + + chunk = len; + - else if (is_nonblocking ()) + - chunk = len = pipe_buf_size; + else + chunk = avail; + +The logic changed such that `len` would no longer be clamped to the same +value as `chunk` in non-blocking mode. Reinstating that clamp in +non-blocking mode would also work around the issue, but it would be a +bit more heavy-handed than the chosen fix. + +Yet another potential fix that was suggested (in +https://github.com/git-for-windows/git/issues/5688#issuecomment-2995952882) +would be to modify the condition when `NtWriteFile()` is skipped so as +to force it to _not_ be skipped when attempting a "short write", i.e. +when `short_write_once != 0`. This fix would have worked around the bug +successfully, but is fixing the bug at the wrong layer (even if it hints +at the fact that the `short_write_once` logic is ill-prepared for +non-blocking mode, but that's a topic for another patch). + +Fixes: cbfaeba4f7 (Cygwin: pipe: Fix incorrect write length in raw_write(), 2024-11-06) +Co-authored-by: Takashi Yano +Signed-off-by: Johannes Schindelin +--- + winsup/cygwin/fhandler/pipe.cc | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc +index 506dd09..169c0bb 100644 +--- a/winsup/cygwin/fhandler/pipe.cc ++++ b/winsup/cygwin/fhandler/pipe.cc +@@ -561,7 +561,7 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len) + ULONG len1; + DWORD waitret = WAIT_OBJECT_0; + +- if (left > chunk && !is_nonblocking ()) ++ if (left > chunk && !real_non_blocking_mode) + len1 = chunk; + else + len1 = (ULONG) left; diff --git a/msys2-runtime/PKGBUILD b/msys2-runtime/PKGBUILD index dead1e5d530..b1a29da0aab 100644 --- a/msys2-runtime/PKGBUILD +++ b/msys2-runtime/PKGBUILD @@ -4,7 +4,7 @@ pkgbase=msys2-runtime pkgname=('msys2-runtime' 'msys2-runtime-devel') pkgver=3.6.3 -pkgrel=2 +pkgrel=3 pkgdesc="Cygwin POSIX emulation engine" arch=('x86_64') url="https://www.cygwin.com/" @@ -75,9 +75,11 @@ source=('msys2-runtime'::git+https://github.com/cygwin/cygwin#tag=cygwin-${pkgve 0046-Change-the-default-base-address-for-x86_64.patch 0047-msys2-runtime-restore-fast-path-for-current-user-pri.patch 0048-Cygwin-Fix-compatibility-with-w32api-headers-v13.patch - 0049-symlink_native-allow-linking-to.patch) + 0049-symlink_native-allow-linking-to.patch + 0050-symlink_native-allow-linking-to.patch + 0051-pipe-fix-SSH-hang-again.patch) sha256sums=('53d2be8a2dcf58e7eae7823ef01a5b8ce0eba704b132d19167396eb162db378c' - '24a6defda6fd286c96e18f8ee8c7f36fd3c010f2df8e3b619e1ebb61bb61b1df' + '0ab4e5dbbcaa665c9adf4c90fb270442b94411bc9a5cc8d6fa8d4418b4857ed3' '5538db757661949423563bab5e4f383dfa4ef0cf3301cf43f23b482392554b3b' '2fb9b2b297797d20cd901eaee2de735e8cdda1c1e5836e9ff77856c0d1216860' '780977d71e35bbe4c0dcda5272895267d68d635f311e224f7981858f7192a85e' @@ -126,7 +128,9 @@ sha256sums=('53d2be8a2dcf58e7eae7823ef01a5b8ce0eba704b132d19167396eb162db378c' '8fbb38dc064d11599b2314311abfa632ec5a8da4786c716299e276abd29d7326' '6db2e2a7e45125974de355fd685d3a652c1f17e5bd2a94606cbba33520d96624' '37f4dcaab659da6f7730a7fdd855c91e51b642bd42820a7f35af514a866db7fd' - 'e2b046866637090bd9123fd2fcd00faac5d903e4b322959b9c13ea1161e786a2') + 'e2b046866637090bd9123fd2fcd00faac5d903e4b322959b9c13ea1161e786a2' + 'b9934e8215605a26ae9b3cac92d3627a1bacad09c83b1329b07bdb3f59a5071d' + '1847922d7dcaca5d16fc39e15cab9183839bde63f5a51d726c34e5a1663ce0e4') # Helper macros to help make tasks easier # apply_patch_with_msg() { @@ -229,7 +233,9 @@ prepare() { 0046-Change-the-default-base-address-for-x86_64.patch \ 0047-msys2-runtime-restore-fast-path-for-current-user-pri.patch \ 0048-Cygwin-Fix-compatibility-with-w32api-headers-v13.patch \ - 0049-symlink_native-allow-linking-to.patch + 0049-symlink_native-allow-linking-to.patch \ + 0050-symlink_native-allow-linking-to.patch \ + 0051-pipe-fix-SSH-hang-again.patch } build() { diff --git a/msys2-runtime/msys2-runtime.commit b/msys2-runtime/msys2-runtime.commit index e48435b8d95..541f0381392 100644 --- a/msys2-runtime/msys2-runtime.commit +++ b/msys2-runtime/msys2-runtime.commit @@ -1 +1 @@ -d973094841109fda63a3e79742dcd06584227e06 +18ec25e018bb30d87109dbeb50727d0bb63a9e81