Re: [BUG] MSVC: error box when interrupting `gitlog` by quitting less

[Johannes Sixt <j.sixt@viscovery.net>]

Please do not cull the Cc list.

Am 3/28/2014 11:07, schrieb Marat Radchenko:
> Jeff King <peff <at> peff.net> writes:
> 
>>
>> I'm not sure what an actual SIGPIPE death looks like on Windows.
> 
> There is no SIGPIPE death on Windows due to total absence of SIGPIPE.
> raise(unsupported int) just causes ugly "git.exe has stopped working"
> window and possibly ends up as SIGABT (I don't know how to check this).

This happens "only" with newer Microsoft C runtime libraries. They do not
return EINVAL (because that usually indicates a bug caused by insufficient
checks before the function call), but crash the program by default in the
way that you observed.

>> What
>> happens if git is still writing data to the pager and the pager exits?
>> Does it receive a signal of some sort?

No; the write attempt returns with EPIPE.

> 
> I'm not sure what you mean, sorry. check_pipe properly detects pager exit.
> The problem is with the way it tries to die.
> 
>> The point of the code in check_pipe is to simulate that death. So
>> whatever happens to git in that case is what we would want to happen
>> when we call raise(SIGPIPE).
> 
> That's what I'm talking about. On Windows, you can't raise(SIGPIPE).
> You can only raise(Windows_supported_signal) where signal is one of:
> SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM as MSDN tells us.

Correct. All other signal number should return EINVAL. But, as I said,
that does not happen by default.

The correct solution is to link against invalidcontinue.obj in the MSVC
build. This is a compiler-provided object file that changes the default
behavior to the "expected" kind, i.e., C runtime functions return EINVAL
when appropriate instead of crashing the application.

>> A possibly simpler option would be to just have the MSVC build skip the
>> raise() call, and do the exit(141) that comes just after. That is
>> probably close enough simulation of SIGPIPE death.

Correct. The MinGW build uses an older C runtime library, which does not
have the strange default behavior, and we do use that exit(141). And with
the fix to the MSVC build suggested above, that version would do likewise.

-- Hannes