Thread (22 messages) 22 messages, 6 authors, 2016-06-15

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

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:56:51

Am 4/16/2013 4:50, schrieb Jeff King:
On Mon, Apr 15, 2013 at 07:34:07PM -0700, Brandon Casey wrote:
quoted
quoted
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).
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help