Skip to content

Commit

Permalink
msys2-runtime: restore blocking mode of read pipe on close()
Browse files Browse the repository at this point in the history
The Cygwin runtime (and for that reason, the MSYS2 runtime, too), change
pipes from blocking to non-blocking, which is a problem when the pipe is
actually opened by something like Git when it asks a credential helper
for information. Git, not expecting the pipe mode to be changed behind
its back, will then error out with "fatal: read error: Invalid
argument".

To address this, this patch was proposed in the Cygwin project as
https://inbox.sourceware.org/cygwin-patches/20240830141553.12128-1-takashi.yano@nifty.ne.jp/

Ideally, Cygwin would not even _try_ to fiddle with the pipe mode,
therefore this patch was not applied as-is. At time of writing, there is
no consensus on any replacement patch, but I have to prepare _something_
for the already-late Git for Windows v2.46.1 (which is likely the last
Git for Windows version to support Windows 7 and Windows 8, hence the
urgency). And this patch at least improves the situation.

This patch will be dropped when Git for Windows will upgrade to MSYS2
runtime v3.5, and hopefully consensus will have been reached about a
better fix by that time.

This commit corresponds to
git-for-windows/msys2-runtime#72 which fixes
git-for-windows/git#5115.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
  • Loading branch information
dscho committed Sep 15, 2024
1 parent f70725e commit 1d376fb
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
From acc71340a481ff3ebe339b015f58ae7505fec585 Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Fri, 30 Aug 2024 23:15:44 +0900
Subject: [PATCH 83/N] Cygwin: pipe: Restore blocking mode of read pipe on
close()

If a cygwin app is executed from a non-cygwin app and the cygwin
app exits, read pipe remains on non-blocking mode because of the
commit fc691d0246b9. Due to this behaviour, the non-cygwin app
cannot read the pipe correctly after that. With this patch, the
blocking mode of the read pipe is stored into was_blocking_read_pipe
on set_pipe_non_blocking() when the cygwin app starts and restored
on close().

Addresses: https://github.com/git-for-windows/git/issues/5115
Fixes: fc691d0246b9 ("Cygwin: pipe: Make sure to set read pipe non-blocking for cygwin apps.");
Reported-by: isaacag, Johannes Schindelin <Johannes.Schindelin@gmx.de>
Reported-at: https://github.com/git-for-windows/git/issues/5115
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
winsup/cygwin/fhandler/pipe.cc | 13 +++++++++++++
winsup/cygwin/local_includes/fhandler.h | 1 +
2 files changed, 14 insertions(+)

diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc
index 8daae8d..c47226b 100644
--- a/winsup/cygwin/fhandler/pipe.cc
+++ b/winsup/cygwin/fhandler/pipe.cc
@@ -54,6 +54,15 @@ fhandler_pipe::set_pipe_non_blocking (bool nonblocking)
IO_STATUS_BLOCK io;
FILE_PIPE_INFORMATION fpi;

+ if (get_device () == FH_PIPER && nonblocking && !was_blocking_read_pipe)
+ {
+ status = NtQueryInformationFile (get_handle (), &io, &fpi, sizeof fpi,
+ FilePipeInformation);
+ if (NT_SUCCESS (status))
+ was_blocking_read_pipe =
+ (fpi.CompletionMode == FILE_PIPE_QUEUE_OPERATION);
+ }
+
fpi.ReadMode = FILE_PIPE_BYTE_STREAM_MODE;
fpi.CompletionMode = nonblocking ? FILE_PIPE_COMPLETE_OPERATION
: FILE_PIPE_QUEUE_OPERATION;
@@ -94,6 +103,8 @@ fhandler_pipe::init (HANDLE f, DWORD a, mode_t mode, int64_t uniq_id)
even with FILE_SYNCHRONOUS_IO_NONALERT. */
set_pipe_non_blocking (get_device () == FH_PIPER ?
true : is_nonblocking ());
+ was_blocking_read_pipe = false;
+
return 1;
}

@@ -675,6 +686,8 @@ fhandler_pipe::close ()
CloseHandle (query_hdl);
if (query_hdl_close_req_evt)
CloseHandle (query_hdl_close_req_evt);
+ if (was_blocking_read_pipe)
+ set_pipe_non_blocking (false);
int ret = fhandler_base::close ();
ReleaseMutex (hdl_cnt_mtx);
CloseHandle (hdl_cnt_mtx);
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
index 15b19f7..f017a6d 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -1193,6 +1193,7 @@ private:
uint64_t pipename_key;
DWORD pipename_pid;
LONG pipename_id;
+ bool was_blocking_read_pipe;
void release_select_sem (const char *);
HANDLE get_query_hdl_per_process (WCHAR *, OBJECT_NAME_INFORMATION *);
HANDLE get_query_hdl_per_system (WCHAR *, OBJECT_NAME_INFORMATION *);
13 changes: 8 additions & 5 deletions msys2-runtime/PKGBUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
pkgbase=msys2-runtime
pkgname=('msys2-runtime' 'msys2-runtime-devel')
pkgver=3.4.10
pkgrel=6
pkgrel=7
pkgdesc="Cygwin POSIX emulation engine"
arch=('x86_64')
url="https://www.cygwin.com/"
Expand Down Expand Up @@ -108,9 +108,10 @@ source=('msys2-runtime'::git+https://github.com/cygwin/cygwin#tag=cygwin-${pkgve
0079-Cygwin-pipe-Make-sure-to-set-read-pipe-non-blocking-.patch
0080-Cygwin-try-to-avoid-recalling-offline-files.patch
0081-Cygwin-get-set-security-descriptors-using-FILE_OPEN_.patch
0082-Cygwin-FILE_OPEN_NO_RECALL-is-incompatible-with-FILE.patch)
0082-Cygwin-FILE_OPEN_NO_RECALL-is-incompatible-with-FILE.patch
0083-Cygwin-pipe-Restore-blocking-mode-of-read-pipe-on-cl.patch)
sha256sums=('4fb94e9f3bab9666dcab50c8940de6e8801d0aae900f0b5a6f79406479e757f1'
'79be406cf3a35561e7873403cb24d84c5cd2c2af71c940319f969b061ac013f1'
'6f297ae4049c2a5b4ff085119284822d1b8045cd24c4b0328ab2808e68c18556'
'351bb1efdbdafe80c981e92d6b425c6ab71c85ce4e990db184e2118158eb2ab6'
'd3d3a01feeae9f7d5e6cb32f4662df74fc9476ff11a1aac3dad2df3e43fd88e4'
'2e50ecd65f2fd413baaf39e5058a6b252245abc7d34f4ebf17dd4f7ffed60ced'
Expand Down Expand Up @@ -192,7 +193,8 @@ sha256sums=('4fb94e9f3bab9666dcab50c8940de6e8801d0aae900f0b5a6f79406479e757f1'
'7b4e8e91b757d3597c85eb2d83a85ed5f57414b74967b0fa4c87a87332eb04c7'
'7012f34047186556ae78b69bfe3eafccb2fe399ab122d5a21cadbf40533b4e0f'
'7fd616f587212716ebdb21cad257c99ec5827638cf7c7819068c1d70020bd251'
'1b74ea7c955991440d61b6059386589e893ddd7513a86cb554833391cf172c41')
'1b74ea7c955991440d61b6059386589e893ddd7513a86cb554833391cf172c41'
'c8542adeb6324ee0edc96c1c9dfce3994cbbe3a42529e63db2b8b7213dbe393f')

# Helper macros to help make tasks easier #
apply_patch_with_msg() {
Expand Down Expand Up @@ -328,7 +330,8 @@ prepare() {
0079-Cygwin-pipe-Make-sure-to-set-read-pipe-non-blocking-.patch \
0080-Cygwin-try-to-avoid-recalling-offline-files.patch \
0081-Cygwin-get-set-security-descriptors-using-FILE_OPEN_.patch \
0082-Cygwin-FILE_OPEN_NO_RECALL-is-incompatible-with-FILE.patch
0082-Cygwin-FILE_OPEN_NO_RECALL-is-incompatible-with-FILE.patch \
0083-Cygwin-pipe-Restore-blocking-mode-of-read-pipe-on-cl.patch
}

build() {
Expand Down
2 changes: 1 addition & 1 deletion msys2-runtime/msys2-runtime.commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2e2ef940151485490aff0c7b00c00965ef7a71a4
bc1f6498ab09182349c5d5daa2d81626990a2832

0 comments on commit 1d376fb

Please sign in to comment.