[pacman-dev] [PATCH] repo-add: Remove lock on SIG{TERM, HUP, QUIT, INT, ERR}

[Lukas Fleischer]

On Wed, Oct 12, 2011 at 04:20:00PM +0200, Lukas Fleischer wrote:
> [...]
> > > The only case where this doesn't work is if we send a
> > > signal when clean_up() is already being executed (it's not too hard to
> > > reproduce that, actually). So just copying the two lines that unlink the
> > > lock file and remove the directory from clean_up() might be the easiest
> > > way to go?
> > I'm not sure we can totally eliminate all race conditions. Can we get
> > closer by unhooking all traps at the start of both trap_exit() and
> > clean_up(), and then explicitly calling clean_up() from trap_exit()?
> 
> I thought of unhooking traps as well but I'm not too enthusiastic about
> that. We `rm -r` a directory in clean_up() and if this ever happens to
> hang for some reason, there will be no way to cancel, except for sending
> SIGKILL.

Forget what I said. +1 to Dan's suggestion. After reconsideration, I
noticed that even if we do not unhook traps, we will get stuck since we
will enter the very same `rm -r` (that might have frozen) again when
sending any signal. The only difference to the first call of clean_up()
is that we will not be able to send the signal again (it's blocked by
the handler invocation) which is exactly the same situation we would
face when unhooking all traps.

Keeping that in mind, there's only a few possible solutions:

* Reduce the instructions in our trap handler to a bare minimum. This is
  what we have in master right now.

* Unhook all traps and hope that nothing will go wrong in our trap
  handler. If we fail here, SIGKILL will be the only way out. Still
  sounds like the most sane alternative to me.

* Register a fallback trap handler when our trap handler is entered, so
  that we can ^C^C if everything goes wrong. Doesn't sound very KISS,
  though.

If there are no objections, I'll take option 2 and implement it exactly
the way Dan suggested (unhook all traps at the start of both trap_exit()
and clean_up(), and then explicitly call clean_up() from trap_exit()).

> 
> > The bash manpage is woefully non-descriptive about signal handlers and
> > their reentry or recall procedure- I'm assuming you could
> > theoretically keep sending signals fast enough that signal handlers
> > pile up on the stack and none of them ever complete, so attempting to
> > unhook as soon as possible and not depend on the signal handlers being
> > in place anymore would at least prevent that stack growth.
> 
> Doing some more tests, it seems like signals are blocked if the signal
> handler for a particular signal is already running.
> 
> So, basically just call clean_up() from trap_exit() (or copy-paste the
> two lines, like I suggested before)?