Skip to content

Commit fcd428f

Browse files
kbleesgitster
authored andcommitted
Win32: fix broken pipe detection
As of "Win32: Thread-safe windows console output", git-log no longer terminates when the pager process dies. This is due to disabling buffering for the replaced stdout / stderr streams. Git-log will periodically fflush stdout (see write_or_die.c/mayble_flush_or_die()), but with no buffering, this is a NOP that always succeeds (so we never detect the EPIPE error). Exchange the original console handles with our console thread pipe handles by accessing the internal MSVCRT data structures directly (which are exposed via __pioinfo for some reason). Implement this with minimal assumptions about the actual data structure to make it work with different (hopefully even future) MSVCRT versions. While messing with internal data structures is ugly, this patch solves the problem at the source instead of adding more workarounds. We no longer need the special winansi_isatty override, and the limitations documented in "Win32: Thread-safe windows console output" are gone (i.e. fdopen(1/2) returns unbuffered streams now, and isatty() for duped console file descriptors works as expected). Signed-off-by: Karsten Blees <[email protected]> Signed-off-by: Stepan Kasal <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent eac14f8 commit fcd428f

File tree

2 files changed

+70
-46
lines changed

2 files changed

+70
-46
lines changed

compat/mingw.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,7 @@ int mingw_raise(int sig);
318318
*/
319319

320320
void winansi_init(void);
321-
int winansi_isatty(int fd);
322321
HANDLE winansi_get_osfhandle(int fd);
323-
#define isatty winansi_isatty
324322

325323
/*
326324
* git specific compatibility

compat/winansi.c

Lines changed: 70 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,6 @@
77
#include <wingdi.h>
88
#include <winreg.h>
99

10-
/*
11-
Functions to be wrapped:
12-
*/
13-
#undef isatty
14-
1510
/*
1611
ANSI codes used by git: m, K
1712
@@ -104,6 +99,7 @@ static int is_console(int fd)
10499

105100
/* initialize attributes */
106101
if (!initialized) {
102+
console = hcon;
107103
attr = plain_attr = sbi.wAttributes;
108104
negative = 0;
109105
initialized = 1;
@@ -465,29 +461,80 @@ static HANDLE duplicate_handle(HANDLE hnd)
465461
return hresult;
466462
}
467463

468-
static HANDLE redirect_console(FILE *stream, HANDLE *phcon, int new_fd)
469-
{
470-
/* get original console handle */
471-
int fd = _fileno(stream);
472-
HANDLE hcon = (HANDLE) _get_osfhandle(fd);
473-
if (hcon == INVALID_HANDLE_VALUE)
474-
die_errno("_get_osfhandle(%i) failed", fd);
475464

476-
/* save a copy to phcon and console (used by the background thread) */
477-
console = *phcon = duplicate_handle(hcon);
465+
/*
466+
* Make MSVCRT's internal file descriptor control structure accessible
467+
* so that we can tweak OS handles and flags directly (we need MSVCRT
468+
* to treat our pipe handle as if it were a console).
469+
*
470+
* We assume that the ioinfo structure (exposed by MSVCRT.dll via
471+
* __pioinfo) starts with the OS handle and the flags. The exact size
472+
* varies between MSVCRT versions, so we try different sizes until
473+
* toggling the FDEV bit of _pioinfo(1)->osflags is reflected in
474+
* isatty(1).
475+
*/
476+
typedef struct {
477+
HANDLE osfhnd;
478+
char osflags;
479+
} ioinfo;
480+
481+
extern __declspec(dllimport) ioinfo *__pioinfo[];
478482

479-
/* duplicate new_fd over fd (closes fd and associated handle (hcon)) */
480-
if (_dup2(new_fd, fd))
481-
die_errno("_dup2(%i, %i) failed", new_fd, fd);
483+
static size_t sizeof_ioinfo = 0;
482484

483-
/* no buffering, or stdout / stderr will be out of sync */
484-
setbuf(stream, NULL);
485-
return (HANDLE) _get_osfhandle(fd);
485+
#define IOINFO_L2E 5
486+
#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E)
487+
488+
#define FDEV 0x40
489+
490+
static inline ioinfo* _pioinfo(int fd)
491+
{
492+
return (ioinfo*)((char*)__pioinfo[fd >> IOINFO_L2E] +
493+
(fd & (IOINFO_ARRAY_ELTS - 1)) * sizeof_ioinfo);
494+
}
495+
496+
static int init_sizeof_ioinfo()
497+
{
498+
int istty, wastty;
499+
/* don't init twice */
500+
if (sizeof_ioinfo)
501+
return sizeof_ioinfo >= 256;
502+
503+
sizeof_ioinfo = sizeof(ioinfo);
504+
wastty = isatty(1);
505+
while (sizeof_ioinfo < 256) {
506+
/* toggle FDEV flag, check isatty, then toggle back */
507+
_pioinfo(1)->osflags ^= FDEV;
508+
istty = isatty(1);
509+
_pioinfo(1)->osflags ^= FDEV;
510+
/* return if we found the correct size */
511+
if (istty != wastty)
512+
return 0;
513+
sizeof_ioinfo += sizeof(void*);
514+
}
515+
error("Tweaking file descriptors doesn't work with this MSVCRT.dll");
516+
return 1;
517+
}
518+
519+
static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
520+
{
521+
ioinfo *pioinfo;
522+
HANDLE old_handle;
523+
524+
/* init ioinfo size if we haven't done so */
525+
if (init_sizeof_ioinfo())
526+
return INVALID_HANDLE_VALUE;
527+
528+
/* get ioinfo pointer and change the handles */
529+
pioinfo = _pioinfo(fd);
530+
old_handle = pioinfo->osfhnd;
531+
pioinfo->osfhnd = new_handle;
532+
return old_handle;
486533
}
487534

488535
void winansi_init(void)
489536
{
490-
int con1, con2, hwrite_fd;
537+
int con1, con2;
491538
char name[32];
492539

493540
/* check if either stdout or stderr is a console output screen buffer */
@@ -516,39 +563,18 @@ void winansi_init(void)
516563
if (atexit(winansi_exit))
517564
die_errno("atexit(winansi_exit) failed");
518565

519-
/* create a file descriptor for the write end of the pipe */
520-
hwrite_fd = _open_osfhandle((long) duplicate_handle(hwrite), _O_BINARY);
521-
if (hwrite_fd == -1)
522-
die_errno("_open_osfhandle(%li) failed", (long) hwrite);
523-
524566
/* redirect stdout / stderr to the pipe */
525567
if (con1)
526-
hwrite1 = redirect_console(stdout, &hconsole1, hwrite_fd);
568+
hconsole1 = swap_osfhnd(1, hwrite1 = duplicate_handle(hwrite));
527569
if (con2)
528-
hwrite2 = redirect_console(stderr, &hconsole2, hwrite_fd);
529-
530-
/* close pipe file descriptor (also closes the duped hwrite) */
531-
close(hwrite_fd);
570+
hconsole2 = swap_osfhnd(2, hwrite2 = duplicate_handle(hwrite));
532571
}
533572

534573
static int is_same_handle(HANDLE hnd, int fd)
535574
{
536575
return hnd != INVALID_HANDLE_VALUE && hnd == (HANDLE) _get_osfhandle(fd);
537576
}
538577

539-
/*
540-
* Return true if stdout / stderr is a pipe redirecting to the console.
541-
*/
542-
int winansi_isatty(int fd)
543-
{
544-
if (fd == 1 && is_same_handle(hwrite1, 1))
545-
return 1;
546-
else if (fd == 2 && is_same_handle(hwrite2, 2))
547-
return 1;
548-
else
549-
return isatty(fd);
550-
}
551-
552578
/*
553579
* Returns the real console handle if stdout / stderr is a pipe redirecting
554580
* to the console. Allows spawn / exec to pass the console to the next process.

0 commit comments

Comments
 (0)