From d2fb621440f513b48b39b52b59033498482f3378 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 3 Jan 2017 18:39:21 +0100 Subject: [PATCH 1/3] mingw: add experimental feature to redirect standard handles On Windows, file handles need to be marked inheritable when they need to be used as standard input/output/error handles for a newly spawned process. The problem with that, of course, is that the "inheritable" flag is global and therefore can wreak havoc with highly multi-threaded applications: other spawned processes will *also* inherit those file handles, despite having *other* input/output/error handles, and never close the former handles because they do not know about them. Let's introduce a set of environment variables (GIT_REDIRECT_STDIN and friends) that point to files, or even better, named pipes and that are used by the spawned Git process. This helps work around above-mentioned issue: those named pipes will be opened in a non-inheritable way upon startup, and no handles are passed around. Signed-off-by: Johannes Schindelin --- compat/mingw.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ t/t0001-init.sh | 7 +++++++ 2 files changed, 52 insertions(+) diff --git a/compat/mingw.c b/compat/mingw.c index f488716f26..c075ad4fce 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -3009,6 +3009,47 @@ static char *wcstoutfdup_startup(char *buffer, const wchar_t *wcs, size_t len) return memcpy(malloc_startup(len), buffer, len); } +static void maybe_redirect_std_handle(const wchar_t *key, DWORD std_id, int fd, + DWORD desired_access, DWORD flags) +{ + DWORD create_flag = fd ? OPEN_ALWAYS : OPEN_EXISTING; + wchar_t buf[MAX_LONG_PATH]; + DWORD max = ARRAY_SIZE(buf); + HANDLE handle; + DWORD ret = GetEnvironmentVariableW(key, buf, max); + + if (!ret || ret >= max) + return; + + /* make sure this does not leak into child processes */ + SetEnvironmentVariableW(key, NULL); + if (!wcscmp(buf, L"off")) { + close(fd); + handle = GetStdHandle(std_id); + if (handle != INVALID_HANDLE_VALUE) + CloseHandle(handle); + return; + } + handle = CreateFileW(buf, desired_access, 0, NULL, create_flag, + flags, NULL); + if (handle != INVALID_HANDLE_VALUE) { + int new_fd = _open_osfhandle((intptr_t)handle, O_BINARY); + SetStdHandle(std_id, handle); + dup2(new_fd, fd); + close(new_fd); + } +} + +static void maybe_redirect_std_handles(void) +{ + maybe_redirect_std_handle(L"GIT_REDIRECT_STDIN", STD_INPUT_HANDLE, 0, + GENERIC_READ, FILE_ATTRIBUTE_NORMAL); + maybe_redirect_std_handle(L"GIT_REDIRECT_STDOUT", STD_OUTPUT_HANDLE, 1, + GENERIC_WRITE, FILE_ATTRIBUTE_NORMAL); + maybe_redirect_std_handle(L"GIT_REDIRECT_STDERR", STD_ERROR_HANDLE, 2, + GENERIC_WRITE, FILE_FLAG_NO_BUFFERING); +} + #if defined(_MSC_VER) #ifdef _DEBUG @@ -3047,6 +3088,8 @@ int msc_startup(int argc, wchar_t **w_argv, wchar_t **w_env) _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF); #endif + maybe_redirect_std_handles(); + /* determine size of argv conversion buffer */ maxlen = wcslen(_wpgmptr); for (k = 1; k < argc; k++) @@ -3111,6 +3154,8 @@ void mingw_startup(void) wchar_t **wenv, **wargv; _startupinfo si; + maybe_redirect_std_handles(); + /* get wide char arguments and environment */ si.newmode = 0; if (__wgetmainargs(&argc, &wargv, &wenv, _CRT_glob, &si) < 0) diff --git a/t/t0001-init.sh b/t/t0001-init.sh index c889716282..7f31acaa2b 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -422,4 +422,11 @@ test_expect_success MINGW 'core.hidedotfiles = false' ' ! is_hidden newdir/.git ' +test_expect_success MINGW 'redirect std handles' ' + GIT_REDIRECT_STDOUT=output.txt git rev-parse --git-dir && + test .git = "$(cat output.txt)" && + test -z "$(GIT_REDIRECT_STDOUT=off git rev-parse --git-dir)" +' + + test_done From 008f14a32394891370d95cbc710b769e0577c856 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 9 Jan 2017 11:12:49 +0100 Subject: [PATCH 2/3] mingw: special-case GIT_REDIRECT_STDERR=2>&1 The "2>&1" notation in POSIX shells implies that stderr is redirected to stdout. Let's special-case this value for the environment variable GIT_REDIRECT_STDERR to allow writing to the same destination as stdout. The functionality was suggested by Jeff Hostetler. Signed-off-by: Johannes Schindelin --- compat/mingw.c | 15 +++++++++++++++ t/t0001-init.sh | 8 +++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/compat/mingw.c b/compat/mingw.c index c075ad4fce..ac690716db 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -3030,6 +3030,21 @@ static void maybe_redirect_std_handle(const wchar_t *key, DWORD std_id, int fd, CloseHandle(handle); return; } + if (std_id == STD_ERROR_HANDLE && !wcscmp(buf, L"2>&1")) { + handle = GetStdHandle(STD_OUTPUT_HANDLE); + if (handle == INVALID_HANDLE_VALUE) { + close(fd); + handle = GetStdHandle(std_id); + if (handle != INVALID_HANDLE_VALUE) + CloseHandle(handle); + } else { + int new_fd = _open_osfhandle((intptr_t)handle, O_BINARY); + SetStdHandle(std_id, handle); + dup2(new_fd, fd); + /* do *not* close the new_fd: that would close stdout */ + } + return; + } handle = CreateFileW(buf, desired_access, 0, NULL, create_flag, flags, NULL); if (handle != INVALID_HANDLE_VALUE) { diff --git a/t/t0001-init.sh b/t/t0001-init.sh index 7f31acaa2b..50a9289059 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -425,7 +425,13 @@ test_expect_success MINGW 'core.hidedotfiles = false' ' test_expect_success MINGW 'redirect std handles' ' GIT_REDIRECT_STDOUT=output.txt git rev-parse --git-dir && test .git = "$(cat output.txt)" && - test -z "$(GIT_REDIRECT_STDOUT=off git rev-parse --git-dir)" + test -z "$(GIT_REDIRECT_STDOUT=off git rev-parse --git-dir)" && + test_must_fail env \ + GIT_REDIRECT_STDOUT=output.txt \ + GIT_REDIRECT_STDERR="2>&1" \ + git rev-parse --git-dir --verify refs/invalid && + printf ".git\nfatal: Needed a single revision\n" >expect && + test_cmp expect output.txt ' From cde7a27524ff8af3e1310207501e6c5ca780f885 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 10 Jan 2017 12:12:50 +0100 Subject: [PATCH 3/3] mingw: document the experimental standard handle redirection This feature is still highly experimental and has not even been contributed to the Git mailing list yet: the feature still needs to be battle-tested more. Signed-off-by: Johannes Schindelin --- Documentation/git.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 02a8190053..679817ef4b 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -1180,6 +1180,23 @@ of clones and fetches. - any external helpers are named by their protocol (e.g., use `hg` to allow the `git-remote-hg` helper) +`GIT_REDIRECT_STDIN`:: +`GIT_REDIRECT_STDOUT`:: +`GIT_REDIRECT_STDERR`:: + (EXPERIMENTAL) Windows-only: allow redirecting the standard + input/output/error handles. This is particularly useful in + multi-threaded applications where the canonical way to pass + standard handles via `CreateProcess()` is not an option because + it would require the handles to be marked inheritable (and + consequently *every* spawned process would inherit them, possibly + blocking regular Git operations). The primary intended use case + is to use named pipes for communication. ++ +Two special values are supported: `off` will simply close the +corresponding standard handle, and if `GIT_REDIRECT_STDERR` is +`2>&1`, standard error will be redirected to the same handle as +standard output. + Discussion[[Discussion]] ------------------------