Skip to content

Commit 1d376fb

Browse files
committed
msys2-runtime: restore blocking mode of read pipe on close()
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/[email protected]/ 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 <[email protected]>
1 parent f70725e commit 1d376fb

File tree

3 files changed

+84
-6
lines changed

3 files changed

+84
-6
lines changed
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
From acc71340a481ff3ebe339b015f58ae7505fec585 Mon Sep 17 00:00:00 2001
2+
From: Takashi Yano <[email protected]>
3+
Date: Fri, 30 Aug 2024 23:15:44 +0900
4+
Subject: [PATCH 83/N] Cygwin: pipe: Restore blocking mode of read pipe on
5+
close()
6+
7+
If a cygwin app is executed from a non-cygwin app and the cygwin
8+
app exits, read pipe remains on non-blocking mode because of the
9+
commit fc691d0246b9. Due to this behaviour, the non-cygwin app
10+
cannot read the pipe correctly after that. With this patch, the
11+
blocking mode of the read pipe is stored into was_blocking_read_pipe
12+
on set_pipe_non_blocking() when the cygwin app starts and restored
13+
on close().
14+
15+
Addresses: https://github.com/git-for-windows/git/issues/5115
16+
Fixes: fc691d0246b9 ("Cygwin: pipe: Make sure to set read pipe non-blocking for cygwin apps.");
17+
Reported-by: isaacag, Johannes Schindelin <[email protected]>
18+
Reported-at: https://github.com/git-for-windows/git/issues/5115
19+
Signed-off-by: Takashi Yano <[email protected]>
20+
Signed-off-by: Johannes Schindelin <[email protected]>
21+
---
22+
winsup/cygwin/fhandler/pipe.cc | 13 +++++++++++++
23+
winsup/cygwin/local_includes/fhandler.h | 1 +
24+
2 files changed, 14 insertions(+)
25+
26+
diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc
27+
index 8daae8d..c47226b 100644
28+
--- a/winsup/cygwin/fhandler/pipe.cc
29+
+++ b/winsup/cygwin/fhandler/pipe.cc
30+
@@ -54,6 +54,15 @@ fhandler_pipe::set_pipe_non_blocking (bool nonblocking)
31+
IO_STATUS_BLOCK io;
32+
FILE_PIPE_INFORMATION fpi;
33+
34+
+ if (get_device () == FH_PIPER && nonblocking && !was_blocking_read_pipe)
35+
+ {
36+
+ status = NtQueryInformationFile (get_handle (), &io, &fpi, sizeof fpi,
37+
+ FilePipeInformation);
38+
+ if (NT_SUCCESS (status))
39+
+ was_blocking_read_pipe =
40+
+ (fpi.CompletionMode == FILE_PIPE_QUEUE_OPERATION);
41+
+ }
42+
+
43+
fpi.ReadMode = FILE_PIPE_BYTE_STREAM_MODE;
44+
fpi.CompletionMode = nonblocking ? FILE_PIPE_COMPLETE_OPERATION
45+
: FILE_PIPE_QUEUE_OPERATION;
46+
@@ -94,6 +103,8 @@ fhandler_pipe::init (HANDLE f, DWORD a, mode_t mode, int64_t uniq_id)
47+
even with FILE_SYNCHRONOUS_IO_NONALERT. */
48+
set_pipe_non_blocking (get_device () == FH_PIPER ?
49+
true : is_nonblocking ());
50+
+ was_blocking_read_pipe = false;
51+
+
52+
return 1;
53+
}
54+
55+
@@ -675,6 +686,8 @@ fhandler_pipe::close ()
56+
CloseHandle (query_hdl);
57+
if (query_hdl_close_req_evt)
58+
CloseHandle (query_hdl_close_req_evt);
59+
+ if (was_blocking_read_pipe)
60+
+ set_pipe_non_blocking (false);
61+
int ret = fhandler_base::close ();
62+
ReleaseMutex (hdl_cnt_mtx);
63+
CloseHandle (hdl_cnt_mtx);
64+
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
65+
index 15b19f7..f017a6d 100644
66+
--- a/winsup/cygwin/local_includes/fhandler.h
67+
+++ b/winsup/cygwin/local_includes/fhandler.h
68+
@@ -1193,6 +1193,7 @@ private:
69+
uint64_t pipename_key;
70+
DWORD pipename_pid;
71+
LONG pipename_id;
72+
+ bool was_blocking_read_pipe;
73+
void release_select_sem (const char *);
74+
HANDLE get_query_hdl_per_process (WCHAR *, OBJECT_NAME_INFORMATION *);
75+
HANDLE get_query_hdl_per_system (WCHAR *, OBJECT_NAME_INFORMATION *);

msys2-runtime/PKGBUILD

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
pkgbase=msys2-runtime
55
pkgname=('msys2-runtime' 'msys2-runtime-devel')
66
pkgver=3.4.10
7-
pkgrel=6
7+
pkgrel=7
88
pkgdesc="Cygwin POSIX emulation engine"
99
arch=('x86_64')
1010
url="https://www.cygwin.com/"
@@ -108,9 +108,10 @@ source=('msys2-runtime'::git+https://github.com/cygwin/cygwin#tag=cygwin-${pkgve
108108
0079-Cygwin-pipe-Make-sure-to-set-read-pipe-non-blocking-.patch
109109
0080-Cygwin-try-to-avoid-recalling-offline-files.patch
110110
0081-Cygwin-get-set-security-descriptors-using-FILE_OPEN_.patch
111-
0082-Cygwin-FILE_OPEN_NO_RECALL-is-incompatible-with-FILE.patch)
111+
0082-Cygwin-FILE_OPEN_NO_RECALL-is-incompatible-with-FILE.patch
112+
0083-Cygwin-pipe-Restore-blocking-mode-of-read-pipe-on-cl.patch)
112113
sha256sums=('4fb94e9f3bab9666dcab50c8940de6e8801d0aae900f0b5a6f79406479e757f1'
113-
'79be406cf3a35561e7873403cb24d84c5cd2c2af71c940319f969b061ac013f1'
114+
'6f297ae4049c2a5b4ff085119284822d1b8045cd24c4b0328ab2808e68c18556'
114115
'351bb1efdbdafe80c981e92d6b425c6ab71c85ce4e990db184e2118158eb2ab6'
115116
'd3d3a01feeae9f7d5e6cb32f4662df74fc9476ff11a1aac3dad2df3e43fd88e4'
116117
'2e50ecd65f2fd413baaf39e5058a6b252245abc7d34f4ebf17dd4f7ffed60ced'
@@ -192,7 +193,8 @@ sha256sums=('4fb94e9f3bab9666dcab50c8940de6e8801d0aae900f0b5a6f79406479e757f1'
192193
'7b4e8e91b757d3597c85eb2d83a85ed5f57414b74967b0fa4c87a87332eb04c7'
193194
'7012f34047186556ae78b69bfe3eafccb2fe399ab122d5a21cadbf40533b4e0f'
194195
'7fd616f587212716ebdb21cad257c99ec5827638cf7c7819068c1d70020bd251'
195-
'1b74ea7c955991440d61b6059386589e893ddd7513a86cb554833391cf172c41')
196+
'1b74ea7c955991440d61b6059386589e893ddd7513a86cb554833391cf172c41'
197+
'c8542adeb6324ee0edc96c1c9dfce3994cbbe3a42529e63db2b8b7213dbe393f')
196198

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

334337
build() {

msys2-runtime/msys2-runtime.commit

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2e2ef940151485490aff0c7b00c00965ef7a71a4
1+
bc1f6498ab09182349c5d5daa2d81626990a2832

0 commit comments

Comments
 (0)