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

[Dan McGee]

On Tue, Oct 11, 2011 at 1:33 PM, Lukas Fleischer
<archlinux at cryptocrack.de> wrote:
> On Tue, Oct 11, 2011 at 07:03:13PM +0200, Lukas Fleischer wrote:
>> On Tue, Oct 11, 2011 at 11:29:31AM -0500, Dan McGee wrote:
>> > On Tue, Oct 11, 2011 at 11:13 AM, Lukas Fleischer
>> > <archlinux at cryptocrack.de> wrote:
>> > > There's no need to keep the lock file if we interrupt repo-{add,remove}
>> > > using ^C (same applies to other signals).
>> > Definite +1 here.
>> >
>> > > In contrast to a clean exit, we do not `rm -rf` the temporary directory
>> > > here, so that it's still possible to analyze what went wrong.
>> > I'm not sure what the right answer is here- I see where you're coming
>> > from, but given that we give the user no indication we're leaving a
>> > mess behind, I think we should either clean it up or tell the user we
>> > did not. Also noticed that we print an error message with TERM even
>> > when HUP and QUIT are caught.
>>
>> I'm not really sure either, but I'd say that showing a message that
>> includes "$tmpdir" might be the best thing to do here.
>>
>> I also submitted a separate patch that fixes the SIGTERM message issue.
>
> Ugh. It's a bit more complicated, even. Given that we use exit at the
> very end of trap_exit(), this should already trigger clean_up()
> automatically.
Ahh, true. Odd I didn't think about this before. So this only happens
when? If someone hits Ctrl-C in quick succession just right?

> 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()?
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.

-Dan