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

non-msys2 programs touching a pipe's read side makes the write side unselectable #4850

Closed
1 task done
wh0 opened this issue Mar 5, 2024 · 10 comments
Closed
1 task done

Comments

@wh0
Copy link

wh0 commented Mar 5, 2024

  • I was not able to find an open or closed issue matching what I'm seeing
  • don't think so, unless a common effect is causing other hangs that are reported

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?

64-bit

$ git --version --build-options

git version 2.44.0.windows.1
cpu: x86_64
built from commit: ad0bbfffa543db6979717be96df630d3e5741331
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
  • Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?

Windows 10 22H2 64-bit

$ cmd.exe /c ver

Microsoft Windows [Version 10.0.19045.4046]
(c) Microsoft Corporation. All rights reserved.
  • What options did you set as part of the installation? Or did you choose the
    defaults?
# One of the following:
> type "C:\Program Files\Git\etc\install-options.txt"
> type "C:\Program Files (x86)\Git\etc\install-options.txt"
> type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt"
> type "$env:USERPROFILE\AppData\Local\Programs\Git\etc\install-options.txt"
$ cat /etc/install-options.txt

Editor Option: Nano
Custom Editor Path:
Default Branch Option:
Path Option: BashOnly
SSH Option: OpenSSH
Tortoise Option: false
CURL Option: OpenSSL
CRLF Option: CRLFCommitAsIs
Bash Terminal Option: MinTTY
Git Pull Behavior Option: Merge
Use Credential Manager: Disabled
Performance Tweaks FSCache: Enabled
Enable Symlinks: Disabled
Enable Pseudo Console Support: Disabled
Enable FSMonitor: Disabled
  • Any other interesting things about your environment that might be related
    to the issue you're seeing?

don't think so 🤞

Details

  • Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

Git Bash

in test3.pl:

#!/usr/bin/perl
my $wfds = '';
vec($wfds, fileno(STDOUT), 1) = 1;
sleep 1;
print STDERR "selecting\n";
select undef, $wfds, undef, undef;
print STDERR "ready\n";
print "abc\n";

then run:

# curl is as shipped with Git for Windows
./test3.pl | (curl --version && cat)
  • What did you expect to occur after running these commands?

the pipe between perl and that subshell should be writable. the select should return and 'abc' should get piped through cat and show up.

curl 8.6.0 (x86_64-w64-mingw32) ...
selecting
ready
abc
  • What actually happened instead?
curl 8.6.0 (x86_64-w64-mingw32) ...
selecting

☝️ and at this point you can't even ctrl+c it

using strace on perl, it ends with

   77  175819 [main] perl 1769 clock_nanosleep: clock_nanosleep (1.000000000)
1003773 1179592 [main] perl 1769 clock_nanosleep: 0 = clock_nanosleep(1, 0, 1.000000000, 0.d)
  314 1179906 [main] perl 1769 time: 1709621915 = time(0x7FFFFCA00)
  741 1180647 [main] perl 1769 fhandler_pty_slave::write: pty0, write(0xA0003FD90, 10)
  256 1180903 [main] perl 1769 fhandler_pty_slave::write: (1273): pty output_mutex (0x190): waiting -1 ms
  246 1181149 [main] perl 1769 fhandler_pty_slave::write: (1273): pty output_mutex: acquired
  244 1181393 [main] perl 1769 fhandler_pty_slave::write: (1289): pty output_mutex(0x190) released
  204 1181597 [main] perl 1769 write: 10 = write(2, 0xA0003FD90, 10)
  224 1181821 [main] perl 1769 pselect: pselect (8, 0x0, 0xA00024A90, 0x0, 0x0, 0x0)
  186 1182007 [main] perl 1769 pselect: to NULL, us -1
  485 1182492 [main] perl 1769 dtable::select_write:  fd 1
  167 1182659 [main] perl 1769 select: sel.always_ready 0
--- Process 4168 (pid: 1769) thread 12824 created
  867 1183526 [pipesel] perl 1769 cygthread::stub: thread 'pipesel', id 0x3218, stack_ptr 0x5FCCB0
  181 1183707 [main] perl 1769 select_stuff::wait: m 3, us 18446744073709551615, wmfo_timeout -1
  235 1183942 [pipesel] perl 1769 SetThreadName: SetThreadDescription() failed. 00000000 10000000

pipesel just never comes back

and what's extra weird:

  1. replacing cat with tee (tee with no args is the same as cat) makes it work
  2. removing the curl makes it work
  3. using curl </dev/null to make curl not touch stdin makes it work
  4. using an msys2 program instead of a non-msys2 program e.g. id instead of curl makes it work
  5. removing the select confirms that nothing weird is breaking the pipe, and the abc shows up properly
  • If the problem was occurring with a specific repository, can you provide the
    URL to that repository to help us with testing?

we're several layers deep, but this "curl and then cat" behavior originated in this script that's meant to run as a ProxyCommand for using Git over SSH:

https://gitlab.com/wh0/te/-/blob/client/te-relay?ref_type=heads#L31

OpenSSH client does the selecting in its client loop. I've replaced it with a script for simplicity.

@rimrul
Copy link
Member

rimrul commented Mar 5, 2024

This sounds like it might be related to msys2/msys2-runtime#202

@dscho
Copy link
Member

dscho commented Mar 5, 2024

@wh0 could you try this in a Portable Git to verify that it exposes the bug, then replace the usr\bin\msys-2.0.dll with the version from the install.zip artifact from this run to verify that the fix that was just applied to Cygwin works for you?

@wh0
Copy link
Author

wh0 commented Mar 6, 2024

Thanks for the link to the msys2 issue, it definitely has elements in common with what I'm seeing. Maybe a difference is that in this issue, it's intended that the external program curl be done before the select. The other side of the pipe is only cat. The subshell running cat also has the file handle, but it's not reading, so I don't know if that counts. They're both msys2 programs.

I see why tee works though, it reads in 1024 byte blocks, while cat reads in 128 KiB blocks. As tyan0 in that thread points, a comment tells us:

... if a
pipe read is currently pending, WriteQuotaAvailable on the write side
is decremented by the number of bytes the read side is requesting.
So it's possible (even likely) that WriteQuotaAvailable is 0, even
if the inbound buffer on the read side is not full. This can lead to
a deadlock situation: The reader is waiting for data, but select
on the writer side assumes that no space is available in the read
side inbound buffer.

@dscho: reproduces in Portable Git. Reproduces after replacing msys-2.0.dll as well.

@dscho
Copy link
Member

dscho commented Mar 6, 2024

Does it also reproduce with plain Cygwin?

@wh0
Copy link
Author

wh0 commented Mar 7, 2024

reproduces in plain Cygwin, when using Windows 10's built in curl.exe. Cygwin also distributes its own built of curl, which doesn't cause this, similar to how using msys2 programs didn't cause this hang either. tested with cygwin 3.5.1-1

@dscho
Copy link
Member

dscho commented Mar 8, 2024

@wh0 would you mind reporting this to the Cygwin project? They are usually very responsive and helpful about clear and complete bug reports.

@rimrul
Copy link
Member

rimrul commented Mar 14, 2024

This has been fixed in cygwin/cygwin@fc691d0 and a Cygwin 3.5.2 is on the horizon, so we'll probably be able to pick this up in time for Git for Windows 2.45.0

@dscho
Copy link
Member

dscho commented Mar 24, 2024

I added a backport of this patch to git-for-windows/msys2-runtime#68. @wh0 would you mind testing after overwriting the msys-2.0.dll in your installation with the one from the install artifact of the PR build?

@wh0
Copy link
Author

wh0 commented Mar 24, 2024

The version with the backport works. 🎉

$ ./test3.pl | (curl --version && cat)
curl 8.6.0 (x86_64-w64-mingw32) ...
selecting
ready
abc

@dscho
Copy link
Member

dscho commented Nov 28, 2024

Since the backport was merged, I assume that this ticket can be closed.

@dscho dscho closed this as completed Nov 28, 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

No branches or pull requests

3 participants