[systemd-devel] [PATCH] service: delayed main PID guessing
Michal Schmidt
mschmidt at redhat.com
Sat Sep 24 03:32:29 PDT 2011
There is a lot of services with incorrectly implemented double-forking.
Their original process exits before the child writes the PID file.
With SysVinit it was rarely a problem, because the PID file was usually
read only when the initscript was called again.
systemd wants to read the PID file after receiving SIGCHLD for the
original process. With a buggy service it may be too early. systemd
warns about the failure to read the PID file and falls back to guessing
the main PID.
Guessing often fails in a bad way for such double-forking services.
systemd is likely to see the first forked child of the service and
assume it is the main PID. This child will fork the second child and
exit itself immediately. systemd will then kill the service.
I see a few options:
(1) Never guess, remove the code completely.
Forking services with incorrect PID file handling would simply not
have their main PID known and systemd would depend solely on the
notification about an empty cgroup as the mechanism to detect the
exit of such services.
(2) Make the guessing opt-in.
Make GuessMainPID=no the default for both native and SysV units.
We could add support for a custom SysV header in the chkconfig
section:
# x-systemd-GuessMainPID: yes
for the benefit of users who like the supervision provided by
systemd, but are unable to fix their services properly for whatever
reason.
(On a tangent... It may be a good idea to allow opting in/out of
main PID supervision in general, even when the PID is known
reliably.)
(3) Improve the guessing so that it is much more likely to give the
right PID.
This is what the patch implements. When no PIDFile= is declared or
when the PID file is missing after the SIGCHLD, give the service
some slack time to put things in order. Then retry the read of the
PID file and if it still fails, do the guessing.
I chose 1 second as the delay. It should be more than enough to
finish the daemonization in the service. It is also on the order
of magnitude that someone doing testing of SysV initscripts by
starting and stopping would have used for sleep in between, thus
masking the bug.
We could actually add (2) in addition to this later.
---
src/service.c | 38 +++++++++++++++++++++++++++-----------
1 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/src/service.c b/src/service.c
index c2053ce..d2b168b 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1318,7 +1318,8 @@ static int service_load_pid_file(Service *s, bool warn_if_missing) {
(unsigned long) s->main_pid, (unsigned long) pid);
service_unwatch_main_pid(s);
s->main_pid_known = false;
- }
+ } else
+ log_debug("Main PID loaded: %lu", (unsigned long) pid);
if ((r = service_set_main_pid(s, pid)) < 0)
return r;
@@ -1349,6 +1350,8 @@ static int service_search_main_pid(Service *s) {
if ((pid = cgroup_bonding_search_main_pid_list(s->meta.cgroup_bondings)) <= 0)
return -ENOENT;
+ log_debug("Main PID guessed: %lu", (unsigned long) pid);
+
if ((r = service_set_main_pid(s, pid)) < 0)
return r;
@@ -1451,6 +1454,7 @@ static void service_set_state(Service *s, ServiceState state) {
if (state != SERVICE_START_PRE &&
state != SERVICE_START &&
state != SERVICE_START_POST &&
+ state != SERVICE_RUNNING &&
state != SERVICE_RELOAD &&
state != SERVICE_STOP &&
state != SERVICE_STOP_SIGTERM &&
@@ -2003,7 +2007,7 @@ fail:
}
static void service_enter_running(Service *s, bool success) {
- int main_pid_ok, cgroup_ok;
+ int main_pid_ok, cgroup_ok, r;
assert(s);
if (!success)
@@ -2012,13 +2016,24 @@ static void service_enter_running(Service *s, bool success) {
main_pid_ok = main_pid_good(s);
cgroup_ok = cgroup_good(s);
+ unit_unwatch_timer(UNIT(s), &s->timer_watch);
+
if ((main_pid_ok > 0 || (main_pid_ok < 0 && cgroup_ok != 0)) &&
- (s->bus_name_good || s->type != SERVICE_DBUS))
+ (s->bus_name_good || s->type != SERVICE_DBUS)) {
+ if (!s->main_pid_known && s->guess_main_pid) {
+ log_debug("Setting up timer to guess main PID in 1 s.");
+ if ((r = unit_watch_timer(UNIT(s), 1*USEC_PER_SEC, &s->timer_watch)) < 0)
+ goto fail;
+ }
service_set_state(s, SERVICE_RUNNING);
- else if (s->remain_after_exit)
+ } else if (s->remain_after_exit)
service_set_state(s, SERVICE_EXITED);
else
service_enter_stop(s, true);
+ return;
+fail:
+ log_warning("%s failed to install guess PID timer: %s", s->meta.id, strerror(-r));
+ service_enter_stop(s, false);
}
static void service_enter_start_post(Service *s) {
@@ -2741,7 +2756,6 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
if (success) {
service_load_pid_file(s, !s->exec_command[SERVICE_EXEC_START_POST]);
- service_search_main_pid(s);
service_enter_start_post(s);
} else
@@ -2750,20 +2764,16 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
break;
case SERVICE_START_POST:
- if (success) {
+ if (success)
service_load_pid_file(s, true);
- service_search_main_pid(s);
- }
s->reload_failure = !success;
service_enter_running(s, true);
break;
case SERVICE_RELOAD:
- if (success) {
+ if (success)
service_load_pid_file(s, true);
- service_search_main_pid(s);
- }
s->reload_failure = !success;
service_enter_running(s, true);
@@ -2820,6 +2830,12 @@ static void service_timer_event(Unit *u, uint64_t elapsed, Watch* w) {
service_enter_stop(s, false);
break;
+ case SERVICE_RUNNING:
+ log_debug("It is time to detect main PID of %s.", u->meta.id);
+ service_load_pid_file(s, true);
+ service_search_main_pid(s);
+ break;
+
case SERVICE_RELOAD:
log_warning("%s operation timed out. Stopping.", u->meta.id);
s->reload_failure = true;
--
1.7.4.4
More information about the systemd-devel
mailing list