Re: [PATCH] t0005: skip signal death exit code test on Windows

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

Am 6/7/2013 12:12, schrieb Erik Faye-Lund:
> On Thu, Jun 6, 2013 at 7:40 PM, Jeff King <peff@peff.net> wrote:
>> On Thu, Jun 06, 2013 at 10:21:47AM -0700, Junio C Hamano wrote:
>>
>>>> The particular deficiency is that when a signal is raise()d whose SIG_DFL
>>>> action will cause process death (SIGTERM in this case), the
>>>> implementation of raise() just calls exit(3).
>>>
>>> After a bit of web searching, it seems to me that this behaviour of
>>> raise() is in msvcrt, and compat/mingw.c::mingw_raise() just calls
>>> that.  In other words, "the implementation of raise()" is at an even
>>> lower level than mingw/msys, and I would agree that it is a platform
>>> issue.
>>
>> Yeah, if it were mingw_raise responsible for this, I would suggest using
>> the POSIX shell "128+sig" instead. We could potentially check for
>> SIG_DFL[1] mingw_raise and intercept and exit there. I don't know if
>> that would create headaches or confusion for other msys programs,
>> though. I'd leave that up to the msysgit people to decide whether it is
>> worth the trouble.
>>
> 
> ...and here's the code to do just that:
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index b295e2f..8b3c1b4 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1573,7 +1573,8 @@ static HANDLE timer_event;
>  static HANDLE timer_thread;
>  static int timer_interval;
>  static int one_shot;
> -static sig_handler_t timer_fn = SIG_DFL, sigint_fn = SIG_DFL;
> +static sig_handler_t timer_fn = SIG_DFL, sigint_fn = SIG_DFL,
> +    sigterm_fn = SIG_DFL;
> 
>  /* The timer works like this:
>   * The thread, ticktack(), is a trivial routine that most of the time
> @@ -1688,6 +1689,10 @@ sig_handler_t mingw_signal(int sig,
> sig_handler_t handler)
>  		sigint_fn = handler;
>  		break;
> 
> +	case SIGTERM:
> +		sigterm_fn = handler;
> +		break;
> +
>  	default:
>  		return signal(sig, handler);
>  	}
> @@ -1715,6 +1720,13 @@ int mingw_raise(int sig)
>  			sigint_fn(SIGINT);
>  		return 0;
> 
> +	case SIGTERM:
> +		if (sigterm_fn == SIG_DFL)
> +			exit(128 + SIGTERM);
> +		else if (sigterm_fn != SIG_IGN)
> +			sigterm_fn(SIGTERM);
> +		return 0;
> +
>  	default:
>  		return raise(sig);
>  	}

That's pointless and does not work. The handler would only be called when
raise() is called, but not when a SIGTERM is received, e.g., via Ctrl-C
from the command line, because that route ends up in MSVCRT, which does
not know about this handler.

If you want to follow this route, you must emulate everything that MSVCRT
already does for us, and that's quite a lot, in particular, when some form
of thread safety is required.

-- Hannes