Hi,
On Wed, Jul 17, 2013 at 03:53:09AM +0200, Lennart Poettering wrote:
> On Wed, 12.06.13 01:22, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
>
> > If ExecStopPost= is defined then it is executed after SIGKILL. Otherwise
> > another round of SIGTERM/SIGSTOP is started which is rather useless when
> > the watchdog timeout hits.
> > So go directly to the final SIGKILL if ExecStopPost= is not defined.
>
> Hmm, why not go always directly into SERVICE_FINAL_SIGKILL? Why bother
> with SERVICE_STOP_SIGKILL at all? What am I missing?
I think a small summery is necessary here:
The original watchdog code just called service_enter_dead(). This had the
problem that no cleanup happened. The process was not killed.
Then I posted a patch to go irectly into SERVICE_FINAL_SIGKILL to fix that.
It got applied.
Then you changed it to SERVICE_STOP_SIGKILL so that ExecStopPost= is still
called.
However, this has a annoying side effect. Consider a service that enters
service_handle_watchdog() with a process that cannot be killed. What
happens is this:
service_handle_watchdog()
STOP_SIGKILL
wait
ExecStopPost= if available
FINAL_SIGTERM
wait
FINAL_SIGKILL
wait
service_enter_dead()
Without a ExecStopPost= defined anything after the first wait is just more
useless waiting.
I wrote this patch to avoid waiting longer than necessary but still be able
to run ExecStopPost=.
I later realized that this problem is not limited to the watchdog usecase.
So I wrote a second patch (it should be applied instead of this one) that
changes the sequence to:
[...]
STOP_SIGKILL
wait
if (ExecStopPost= is available) {
ExecStopPost=
FINAL_SIGTERM
wait
FINAL_SIGKILL
wait
}
service_enter_dead()
I sent it as a reply to the first patch. I've also attached it for
reference.
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>From 86efc9efe5a365de219457d033844cb1022fdacb Mon Sep 17 00:00:00 2001
From: Michael Olbrich <m.olbr...@pengutronix.de>
Date: Wed, 12 Jun 2013 08:41:16 +0200
Subject: [PATCH] service: don't enter a second SIGTERM/SIGKILL cycle if no
ExecStopPost= process is defined
It won't help if the main process is still there and there is no new
process to kill.
Signed-off-by: Michael Olbrich <m.olbr...@pengutronix.de>
---
src/core/service.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/core/service.c b/src/core/service.c
index 2bc0dc5..b98f11a 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1987,7 +1987,7 @@ static void service_enter_stop_post(Service *s, ServiceResult f) {
service_set_state(s, SERVICE_STOP_POST);
} else
- service_enter_signal(s, SERVICE_FINAL_SIGTERM, SERVICE_SUCCESS);
+ service_enter_dead(s, SERVICE_SUCCESS, true);
return;
--
1.8.3.2
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
