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

Fix SSH hangs #75

Merged
merged 1 commit into from
Oct 10, 2024
Merged

Fix SSH hangs #75

merged 1 commit into from
Oct 10, 2024

Conversation

dscho
Copy link
Member

@dscho dscho commented Oct 10, 2024

Since upgrading the MSYS2 runtime to v3.5.4, cloning or fetching via SSH is hanging indefinitely.

Bisecting the problem points to 555afcb (Cygwin: select: set pipe writable only if PIPE_BUF bytes left, 2024-08-18), which we hereby essentially revert.

This fixes git-for-windows/git#5199.

It was reported in git-for-windows/git#5199
that as of v3.5.4, cloning or fetching via SSH is hanging indefinitely.

Bisecting the problem points to 555afcb (Cygwin: select: set pipe
writable only if PIPE_BUF bytes left, 2024-08-18). That commit's
intention seems to look at the write buffer, and only report the pipe as
writable if there are more than one page (4kB) available.

However, the number that is looked up is the number of bytes that are
already in the buffer, ready to be read, and further analysis
shows that in the scenario described in the report, the number of
available bytes is substantially below `PIPE_BUF`, but as long as they
are not handled, there is apparently a dead-lock.

Since the old logic worked, and the new logic causes a dead-lock, let's
essentially revert 555afcb (Cygwin: select: set pipe writable only if
PIPE_BUF bytes left, 2024-08-18).

Note: This is not a straight revert, as the code in question has been
modified subsequently, and trying to revert the original commit would
cause merge conflicts. Therefore, the diff looks very different from the
reverse diff of the commit whose logic is reverted.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho self-assigned this Oct 10, 2024
@dscho dscho marked this pull request as ready for review October 10, 2024 18:28
@dscho
Copy link
Member Author

dscho commented Oct 10, 2024

/open pr

The workflow run was started

@Saibamen
Copy link

Saibamen commented Oct 10, 2024

Can we have some integration tests for this, please?
I think a few developers have lost a lot of time @ work because of this regression, including me - I have new laptop since 2 days, and I was thinking this is because of bug after upgrading Win10 to Win 11, strange bug in Azure DevOps or missing/wrong configurations (everything worked on my old laptop).

@dscho
Copy link
Member Author

dscho commented Oct 10, 2024

Can we have some integration tests for this, please?

Yes, as soon as you write them, test them, and open a PR.

dscho added a commit to git-for-windows/git-for-windows-automation that referenced this pull request Oct 10, 2024
@dscho dscho merged commit 7913a41 into git-for-windows:main Oct 10, 2024
2 checks passed
@dscho dscho deleted the fix-ssh-hangs branch October 10, 2024 20:48
vszakats added a commit to curl/curl that referenced this pull request Oct 23, 2024
Fix the significant perf regression for vcpkg jobs by switching to the
MSYS2 shell environment from Git for Windows. This env is already used
for old-mingw-w64 job that remained unaffected by this issue.

The issue began with the windows-runner update 20241015.1.0. It bumped
Git for Windows from Git 2.46.2.windows.1 to Git 2.47.0.windows.1. GfW
bumped its MSYS2 components, including `msys-2.0.dll`. That's Cygwin
code, which may have contributed to this. Pipes were involved and
`runtests.pl` relies on pipes heavily in parallel mode. (The issue was
not seen with parallel tests disabled, in retrospect.)

This is useful as a permanent solution too. It drop GfW as a dependency
and makes Windows jobs use one less shell/env flavour.

Long term it might help to use native Windows Perl to avoid the MSYS
layer completely, if there is a way to make that work.

Assortment of possibly related links:
https://cygwin.com/pipermail/cygwin/2024-August/256398.html
cygwin/cygwin@f78009c
cygwin/cygwin@7f3c225

actions/runner-images#10843
git-for-windows/git#5199
git-for-windows/msys2-runtime#75
git-for-windows/msys2-runtime@7913a41
git-for-windows/msys2-runtime@555afcb
cygwin/cygwin@1c5f4dc

Follow-up to c33174d #15364
Follow-up to 1e03059 #15356

Closes #15380
dscho added a commit to dscho/msys2-runtime that referenced this pull request Dec 24, 2024
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.

[SOLVED in v2.47.0(2)] v2.47.0 Git hangs on fetch
3 participants