Skip to content

Commit

Permalink
fixup! mingw: spawned processes need to inherit only standard handles
Browse files Browse the repository at this point in the history
On Windows 7 and older, the API to restrict which file handles get
to be inherited by a spawned process is a bit peculiar. For example,
when launching `git ls-remote https://...` in a Git Bash, everything
works as expected. But when launching that in a Git CMD,
CreateProcessW() will fail (with an ERROR_NO_SYSTEM_RESOURCES) if even
one of the standard handles are marked to be inherited specifically.

Apparently, it has something to do with handles' file types and with
cmd.exe setting up its standard handles in a specific way.

Rather than trying to guess correctly under which circumstances we
should, or should not, list those standard handles, let's just work
around this by detecting that particular error and simply trying to call
CreateProcessW() again, this time without any special thread attribute
list to restrict which handles get to be inherited by the spawned
process.

This means that we potentially run into more locking problems on Windows
7 and older than we would like, but it is still better than to deny Git
to spawn any child processes.

This fixes #1475

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
  • Loading branch information
dscho committed Feb 5, 2018
1 parent 60d8d6e commit dc364ab
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions compat/mingw.c
Original file line number Diff line number Diff line change
Expand Up @@ -1593,7 +1593,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
const char *dir, const char *prepend_cmd,
int fhin, int fhout, int fherr)
{
static int atexit_handler_initialized;
static int atexit_handler_initialized, restrict_handle_inheritance = 1;
STARTUPINFOEXW si;
PROCESS_INFORMATION pi;
LPPROC_THREAD_ATTRIBUTE_LIST attr_list = NULL;
Expand Down Expand Up @@ -1722,7 +1722,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
wenvblk = make_environment_block(deltaenv);

memset(&pi, 0, sizeof(pi));
if (stdhandles_count &&
if (restrict_handle_inheritance && stdhandles_count &&
(InitializeProcThreadAttributeList(NULL, 1, 0, &size) ||
GetLastError() == ERROR_INSUFFICIENT_BUFFER) &&
(attr_list = (LPPROC_THREAD_ATTRIBUTE_LIST)
Expand All @@ -1741,6 +1741,25 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
stdhandles_count ? TRUE : FALSE,
flags, wenvblk, dir ? wdir : NULL,
&si.StartupInfo, &pi);

/*
* On Windows 2008 R2, it seems that specifying certain types of handles
* (such as FILE_TYPE_CHAR or FILE_TYPE_PIPE) will always produce an
* error. Rather than playing finicky and fragile games, let's just try
* to detect this situation and simply try again without restricting any
* handle inheritance. This is still better than failing to create
* processes.
*/
if (!ret && GetLastError() == ERROR_NO_SYSTEM_RESOURCES &&
restrict_handle_inheritance && stdhandles_count) {
restrict_handle_inheritance = 0;
flags &= ~EXTENDED_STARTUPINFO_PRESENT;
ret = CreateProcessW(*wcmd ? wcmd : NULL, wargs, NULL, NULL,
stdhandles_count ? TRUE : FALSE,
flags, wenvblk, dir ? wdir : NULL,
&si.StartupInfo, &pi);
}

if (!ret)
errno = err_win_to_posix(GetLastError());

Expand Down

0 comments on commit dc364ab

Please sign in to comment.