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

Cygwin: pipe: Fix hang due to inadvertent 0-length raw_write() and some signal fixes #91

Merged
merged 3 commits into from
Mar 31, 2025

Conversation

dscho
Copy link
Member

@dscho dscho commented Mar 27, 2025

This is a companion of msys2/msys2-runtime#271 and of https://inbox.sourceware.org/cygwin-patches/40a66dcf272eff407794bb94616d5b8ba833fafa.1743076896.git.johannes.schindelin@gmx.de/.

It also backports d71aeccff4d219b1cc7a6b0d17dcea7e5bb1b2e9 and 7f67575711f91ee0b738f836e2ed93c2e41b5248.

This closes git-for-windows/git#5521 and git-for-windows/git#5522.

@dscho dscho self-assigned this Mar 27, 2025
@dscho
Copy link
Member Author

dscho commented Mar 31, 2025

/open pr

The workflow run was started

@dscho
Copy link
Member Author

dscho commented Mar 31, 2025

I just merged msys2/msys2-runtime#271, so let's deploy the fix to Git for Windows' Pacman repository, too.

dscho added 2 commits March 31, 2025 08:27
It is possible for `NtQueryInformationFile()` to report a 0-length
`InboundQuota` when obtaining `FilePipeLocalInformation`. This seems to
be the case e.g. when a pipe was created on the other side and its quota
information is not available on the client side.

This can lead to a situation where the `avail` variable is set to 0, and
since that is used to cap the number of bytes to send, a 0-length write.
Which hangs forever.

This was observed in the MSYS2 project when building GIMP, and reduced
to a simple test case where a MINGW `ninja.exe` tries to call an MSYS
`bison.exe` and the error message (saying that `bison` wants to have
some input) is not even shown.

Since the minimal pipe buffer size is 4k, let's ensure that it is at
least that, even when `InboundQuota` reports 0.

This fixes msys2/msys2-runtime#270

Fixes: cbfaeba (Cygwin: pipe: Fix incorrect write length in raw_write())
Helped-by: Corinna Vinschen <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
x86_64 ABI requires the direction flag in CPU flags register cleared.
https://learn.microsoft.com/en-us/cpp/build/x64-software-conventions
However, currently that flag is not maintained in signal handler.
Therefore, if the signal handler is called when that flag is set, it
destroys the data and may crash if rep instruction is used in the
signal handler. With this patch, the direction flag is cleared in
sigdelayed() by adding cld instruction.

Backported-from: d71aeccff4 (Cygwin: signal: Clear direction flag in sigdeleyed, 2025-03-24)
Addresses: https://cygwin.com/pipermail/cygwin/2025-March/257704.html
Fixes: 1fd5e00 ("import winsup-2000-02-17 snapshot")
Reported-by: Christian Franke <[email protected]>
Reviewed-by: Corinna Vischen <[email protected]>
Signed-off-by: Takashi Yano <[email protected]>
(cherry picked from commit 9f4cec47df66a913256912c358afe5e597dec25c)
Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho changed the title Cygwin: pipe: Fix hang due to inadvertent 0-length raw_write() Cygwin: pipe: Fix hang due to inadvertent 0-length raw_write() and some signal fixes Mar 31, 2025
After the commit 0210c77, the context passed to signal handler
cannot be accessed from the signal handler that uses alternate stack.
This is because the context locally copied is on the stack that is
different area from the signal handler uses. With this patch, copy
the context to alternate signal stack area to avoid this situation.

Backported-from: 7f67575711 (Cygwin: signal: Copy context to alternate stack in the SA_ONSTACK case, 2025-03-25)
Addresses: https://cygwin.com/pipermail/cygwin/2025-March/257714.html
Fixes: 0210c77 ("Cygwin: signal: Use context locally copied in call_signal_handler()")
Reported-by: Bruno Haible <[email protected]>
Reviewed-by: Corinna Vischen <[email protected]>
Signed-off-by: Takashi Yano <[email protected]>
(cherry picked from commit 0d0e76b99025704d8ee44a44b19a23af9aafe297)
Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member Author

dscho commented Mar 31, 2025

/open pr

The workflow run was started

@dscho dscho merged commit 21630fd into git-for-windows:main Mar 31, 2025
28 of 41 checks passed
@dscho dscho deleted the fix-ninja-hang branch March 31, 2025 07:59
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.

[New msys2-runtime-package version] msys2-runtime: backport pipe hang fix
1 participant