git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johannes Sixt <j.sixt@viscovery.net>
Date
Jun 7, 2013, 12:19 UTC
Message-ID
<51B1CFD4.3030908@viscovery.net>
In-Reply-To
<CABPQNSYE=Mvrmc44dZmKnB14KLh4A=HxWo2-xgnJRyj1Q+BJLg@mail.gmail.com>
Am 6/7/2013 14:00, schrieb Erik Faye-Lund:
Show 76 quoted lines
> On Fri, Jun 7, 2013 at 12:24 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> 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.
> 
> 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
Previous: Erik Faye-LundNext: Erik Faye-Lund
Message 91 of 104 in “What's cooking in git.git (Jun 2013, #02; Tue, 4)”
  1. Junio C HamanoJun 4, 2013
  2. [Administrivia] On ruby and contrib/Junio C Hamano, Jun 5, 2013
  3. David LangJun 5, 2013
  4. Felipe ContrerasJun 5, 2013
  5. Michael HaggertyJun 5, 2013
  6. Junio C HamanoJun 5, 2013
  7. Matthieu MoyJun 6, 2013
  8. Junio C HamanoJun 7, 2013
  9. Thomas Ferris NicolaisenJun 6, 2013
  10. Felipe ContrerasJun 5, 2013
  11. demerphqJun 6, 2013
  12. Felipe ContrerasJun 6, 2013
  13. Barry FishmanJun 6, 2013
  14. Felipe ContrerasJun 6, 2013
  15. Barry FishmanJun 6, 2013
  16. Felipe ContrerasJun 6, 2013
  17. Barry FishmanJun 6, 2013
  18. Felipe ContrerasJun 6, 2013
  19. Charles McGarveyJun 6, 2013
  20. Greg TroxelJun 6, 2013
  21. Felipe ContrerasJun 6, 2013
  22. David LangJun 6, 2013
  23. Felipe ContrerasJun 6, 2013
  24. Ramkumar RamachandraJun 6, 2013
  25. David LangJun 6, 2013
  26. Ramkumar RamachandraJun 7, 2013
  27. Junio C HamanoJun 7, 2013
  28. Ramkumar RamachandraJun 7, 2013
  29. Felipe ContrerasJun 7, 2013
  30. Ramkumar RamachandraJun 7, 2013
  31. Ramkumar RamachandraJun 7, 2013
  32. Johannes SchindelinJun 9, 2013
  33. Ramkumar RamachandraJun 9, 2013
  34. Junio C HamanoJun 9, 2013
  35. Felipe ContrerasJun 10, 2013
  36. Junio C HamanoJun 7, 2013
  37. Felipe ContrerasJun 7, 2013
  38. Duy NguyenJun 8, 2013
  39. Felipe ContrerasJun 8, 2013
  40. Duy NguyenJun 8, 2013
  41. Felipe ContrerasJun 8, 2013
  42. Felipe ContrerasJun 7, 2013
  43. Greg TroxelJun 6, 2013
  44. Felipe ContrerasJun 6, 2013
  45. Ramkumar RamachandraJun 6, 2013
  46. Dependencies and packaging (Re: [Administrivia] On ruby and contrib/)Jonathan Nieder, Jun 6, 2013
  47. Felipe ContrerasJun 7, 2013
  48. Johannes SchindelinJun 6, 2013
  49. Ramkumar RamachandraJun 6, 2013
  50. Johannes SchindelinJun 7, 2013
  51. Ramkumar RamachandraJun 7, 2013
  52. Matthieu MoyJun 7, 2013
  53. Ramkumar RamachandraJun 7, 2013
  54. Ramkumar RamachandraJun 7, 2013
  55. Matthieu MoyJun 7, 2013
  56. Ramkumar RamachandraJun 7, 2013
  57. Matthieu MoyJun 7, 2013
  58. Felipe ContrerasJun 7, 2013
  59. Jonathan NiederJun 7, 2013
  60. Matthew RuffaloJun 7, 2013
  61. Junio C HamanoJun 7, 2013
  62. Felipe ContrerasJun 7, 2013
  63. Ramkumar RamachandraJun 7, 2013
  64. Johannes SchindelinJun 9, 2013
  65. Duy NguyenJun 8, 2013
  66. Felipe ContrerasJun 8, 2013
  67. Duy NguyenJun 8, 2013
  68. Felipe ContrerasJun 8, 2013
  69. Duy NguyenJun 8, 2013
  70. Felipe ContrerasJun 8, 2013
  71. Jeff KingJun 8, 2013
  72. Felipe ContrerasJun 8, 2013
  73. Jeff KingJun 9, 2013
  74. Felipe ContrerasJun 9, 2013
  75. Jeff KingJun 9, 2013
  76. Felipe ContrerasJun 9, 2013
  77. Johannes SchindelinJun 9, 2013
  78. Johannes SixtJun 5, 2013
  79. Jeff KingJun 5, 2013
  80. t0005: skip signal death exit code test on WindowsJohannes Sixt, Jun 6, 2013
  81. Jeff KingJun 6, 2013
  82. Felipe ContrerasJun 6, 2013
  83. Jeff KingJun 6, 2013
  84. Felipe ContrerasJun 6, 2013
  85. Junio C HamanoJun 6, 2013
  86. Jeff KingJun 6, 2013
  87. Johannes SixtJun 7, 2013
  88. Erik Faye-LundJun 7, 2013
  89. Johannes SixtJun 7, 2013
  90. Erik Faye-LundJun 7, 2013
  91. Johannes SixtJun 7, 2013
  92. Erik Faye-LundJun 7, 2013
  93. Johannes SixtJun 7, 2013
  94. Erik Faye-LundJun 7, 2013
  95. mingw: make mingw_signal return the correct handlerJohannes Sixt, Jun 10, 2013
  96. Erik Faye-LundJun 10, 2013
  97. Junio C HamanoJun 10, 2013
  98. Jeff KingJun 9, 2013
  99. Junio C HamanoJun 9, 2013
  100. Johannes SixtJun 10, 2013
  101. Erik Faye-LundJun 10, 2013
  102. Felipe ContrerasJun 6, 2013
  103. Erik Faye-LundJun 7, 2013
  104. Erik Faye-LundJun 7, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.