Re: [PATCH v3 06/10] run-command: add clean_on_exit_handler

On 02 Aug 2016, at 07:53, Johannes Sixt [off-list ref] wrote:

Am 01.08.2016 um 13:14 schrieb Lars Schneider:

quoted

quoted

On 30 Jul 2016, at 11:50, Johannes Sixt [off-list ref] wrote:
Am 30.07.2016 um 01:37 schrieb larsxschneider@gmail.com:

quoted

static struct child_to_clean *children_to_clean;

@@ -30,6 +31,8 @@ static void cleanup_children(int sig, int in_signal)

{
	while (children_to_clean) {
		struct child_to_clean *p = children_to_clean;
+		if (p->clean_on_exit_handler)
+			p->clean_on_exit_handler(p->pid);

This summons demons. cleanup_children() is invoked from a signal
handler. In this case, it can call only async-signal-safe functions.
It does not look like the handler that you are going to install
later will take note of this caveat!

quoted

		children_to_clean = p->next;
		kill(p->pid, sig);
		if (!in_signal)

The condition that we see here in the context protects free(p)
(which is not async-signal-safe). Perhaps the invocation of the new
callback should be skipped in the same manner when this is called
from a signal handler? 507d7804 (pager: don't use unsafe functions
in signal handlers) may be worth a look.

Thanks a lot of pointing this out to me!

Do I get it right that after the signal "SIGTERM" I can do a cleanup
and don't need to worry about any function calls but if I get any
other signal then I can only perform async-signal-safe calls?

No. SIGTERM is not special.

Perhaps you were misled by the SIGTERM mentioned in cleanup_children_on_exit()? This function is invoked on regular exit (not from a signal). SIGTERM is used in this case to terminate children that are still lingering around.

Yes, that was my source of confusion. Thanks for the clarification!

quoted

If this is correct, then the following solution would work great:

		if (!in_signal && p->clean_on_exit_handler)
			p->clean_on_exit_handler(p->pid);

This should work nevertheless because in_signal is set when the function is invoked from a signal handler (of any signal that is caught) via cleanup_children_on_signal().

Right. Thank you!

- Lars