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

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Mar 28, 2014, 10:28 UTC
Message-ID: <53354EE3.2050908@viscovery.net>
In-Reply-To: <loom.20140328T105136-494@post.gmane.org>

Please do not cull the Cc list.

Am 3/28/2014 11:07, schrieb Marat Radchenko:

Show 8 quoted lines

> 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.

Show 11 quoted lines

> 
> 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