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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 6/7/2013 14:00, schrieb Erik Faye-Lund:
> On Fri, Jun 7, 2013 at 12:24 PM, Johannes Sixt <j.sixt@xxxxxxxxxxxxx> wrote:
>> Am 6/7/2013 12:12, schrieb Erik Faye-Lund:
>>> On Thu, Jun 6, 2013 at 7:40 PM, Jeff King <peff@xxxxxxxx> 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.
> 
> That's not entirely true. On Windows, there's only *one* way to
> generate SIGTERM; "signal(SIGTERM)". Ctrl+C does not generate SIGTERM.
> We generate SIGINT on Ctrl+C in mingw_fgetc, but the default Control+C
> handler routine calls ExitProcess():
> http://msdn.microsoft.com/en-us/library/windows/desktop/ms683242(v=vs.85).aspx

But a call to signal(SIGTERM, my_handler) should divert Ctrl+C to
my_handler. The unpatched version does, because MSVCRT now knows about
my_handler and sets things up so that the event handler calls my_handler.
But your patched version bypasses MSVCRT, and the default (whatever MSVCRT
has set up) happens, and my_handler is not called.

-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]