Re: glibc mutex deadlock in signal handler

[Takashi Iwai <tiwai@suse.de>]

On Fri, 04 Sep 2015 15:04:48 +0200,
Jeff King wrote:
> 
> On Fri, Sep 04, 2015 at 11:35:57AM +0200, Takashi Iwai wrote:
> 
> > > Hmm, is there is any reason to just pass an "in_signal" flag to
> > > wait_for_pager(), to avoid duplicating the logic?
> > 
> > Just because wait_for_pager() itself is an atexit hook that can't take
> > an argument, so we'd need to split to a new function.  I don't mind
> > either way.  The revised patch is below.
> 
> Ah, right. That's unfortunate, but I think I prefer adding the extra
> wrapper to duplicating the contents of the function.
> 
> > -- 8< --
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH v2] pager: don't use unsafe functions in signal handlers
> > [...]
> 
> This looks good to me. Do you plan on fixing any of the other handlers
> (you don't have to; I just want to know if somebody is planning to work
> on it).

Heh, I'd like to leave the rest for professionals :)

> The pattern of atexit and signal handlers is repeated in several places,
> and it seems like we will have to add the same in_signal boilerplate in
> each instance. I wonder if we should provide a global "register_cleanup"
> that takes a "void (*func)(int in_signal))" function pointer, and:
> 
>   1. Adds it to a list (ideally in a way that is atomic if we get
>      interrupted while adding to the list).
> 
>   2. If not already run, registers an atexit() handler and
>      sigchain_push_common for a meta-handler which runs through the list
>      and runs each handler.
> 
> It's not a _huge_ amount of boilerplate code we'd be saving, but at
> least conforming to the "in_signal" function template would make people
> think twice about what they're doing inside the cleanup function. :)

Or, we may have a global in_signal flag.  Of course, this is bad if
git itself is multi-threaded, though.


thanks,

Takashi