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

Re: [PATCH 1/3] usage: refactor die-recursion checks

From
Johannes Sixt <j.sixt@viscovery.net>
Date
Apr 16, 2013, 07:18 UTC
Message-ID
<516CFB56.1090805@viscovery.net>
In-Reply-To
<20130416025024.GA20932@sigill.intra.peff.net>
Am 4/16/2013 4:50, schrieb Jeff King:
Show 13 quoted lines
> On Mon, Apr 15, 2013 at 07:34:07PM -0700, Brandon Casey wrote:
> 
>>> Right. My assumption was that we are primarily interested in protecting
>>> against the die_routine. Compat functions should never be calling die.
>>
>> I think the rule we have been enforcing is less strict than that.  We
>> have only said that any compat function in a die handler path should
>> never call die.  But maybe that's what you meant.
> 
> No, I assumed we were following the stronger rule. If you are a compat
> function for a C library function, then you should never need to die.
> You should be conforming to the existing interface, which will have some
> mechanism for passing back an error.

This rule has been violated LOOOONG ago, and not only in compat/mingw.c (see xmalloc in compat/qsort.c, for example).

Show 8 quoted lines
>> The primary motivation was that Hannes Sixt had to step in and point
>> out yet again that the high-level memory allocators should not be
>> called in anything that could be in a die handler code path.  I was on
>> the thread, but I don't remember the topic (ah, Jonathan has stepped
>> in with the answer).  I do remember that I was not the only one who
>> had forgotten about that rule though.
> 
> Yeah, it is subtle enough that it may be worth protecting against.
Too late.
Show 27 quoted lines
>> To implement this check correctly/completely (i.e. detect recursion in
>> the main thread as well as in any child threads), I think you really
>> do need to use thread-local storage as you mentioned in 3/3 which
>> could look something like:
>>
>>    static pthread_key_t dying;
>>    static pthread_once_t dying_once = PTHREAD_ONCE_INIT;
>>
>>    void setup_die_counter(void)
>>    {
>>            pthread_key_create(&dying, NULL);
>>    }
>>
>>    check_die_recursion(void)
>>    {
>>            pthread_once(&dying_once, setup_die_counter);
>>            if (pthread(getspecific(dying)) {
>>                    puts("BUG: recursion...");
>>                    exit(128);
>>            }
>>
>>            pthread_setspecific(dying, &dying);
>>    }
> 
> Yeah, that seems sane; my biggest worry was that it would create
> headaches for Windows folks, who would have to emulate pthread_key. But
> it seems like we already added support in 9ba604a.

pthread_key is not a problem, but pthread_once is. It's certainly solvable, but do we really have to?

-- Hannes
Previous: Jeff KingNext: Jeff King
Message 10 of 23 in “avoid bogus "recursion detected in die handler" message”
  1. 0/3 avoid bogus "recursion detected in die handler" messageJeff King, Apr 15, 2013
  2. 1/3 usage: refactor die-recursion checksJeff King, Apr 15, 2013
  3. Eric SunshineApr 15, 2013
  4. Jeff KingApr 15, 2013
  5. Brandon CaseyApr 16, 2013
  6. Jeff KingApr 16, 2013
  7. Jonathan NiederApr 16, 2013
  8. Brandon CaseyApr 16, 2013
  9. Jeff KingApr 16, 2013
  10. Johannes SixtApr 16, 2013
  11. Jeff KingApr 16, 2013
  12. Johannes SixtApr 16, 2013
  13. 0/2 avoid bogus "recursion detected in die handler" messageJeff King, Apr 16, 2013
  14. 1/2 usage: allow pluggable die-recursion checksJeff King, Apr 16, 2013
  15. 2/2 run-command: use thread-aware die_is_recursing routineJeff King, Apr 16, 2013
  16. Junio C HamanoApr 16, 2013
  17. Jonathan NiederApr 17, 2013
  18. Jeff KingApr 17, 2013
  19. Erik Faye-LundApr 23, 2013
  20. 2/3 run-command: factor out running_main_thread functionJeff King, Apr 15, 2013
  21. Jonathan NiederApr 16, 2013
  22. Jeff KingApr 16, 2013
  23. 3/3 usage: do not check die recursion outside main threadJeff King, Apr 15, 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.