service: watch main pid even in final states

In some circumstances, for example when start-up times out we
immediately jump into the final state, at which point we still should
try to watch the main pid so that the SIGCHLD allows us to quickly
move into dead state.

diff --git a/src/core/service.c b/src/core/service.c
index 67cf63099ad498a4b51217e543e105c1e921d11b..fa470619389b1b157e27ba332023acaa5f5685be 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1488,7 +1488,8 @@ static void service_set_state(Service *s, ServiceState state) {
         if (!IN_SET(state,
                     SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST,
                     SERVICE_RELOAD,
         if (!IN_SET(state,
                     SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST,
                     SERVICE_RELOAD,

-                    SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST,
+                    SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL,
+                    SERVICE_STOP_POST,

                     SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL,
                     SERVICE_AUTO_RESTART))
                 s->timer_event_source = sd_event_source_unref(s->timer_event_source);
                     SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL,
                     SERVICE_AUTO_RESTART))
                 s->timer_event_source = sd_event_source_unref(s->timer_event_source);


@@ -1496,7 +1497,9 @@ static void service_set_state(Service *s, ServiceState state) {
         if (!IN_SET(state,
                     SERVICE_START, SERVICE_START_POST,
                     SERVICE_RUNNING, SERVICE_RELOAD,
         if (!IN_SET(state,
                     SERVICE_START, SERVICE_START_POST,
                     SERVICE_RUNNING, SERVICE_RELOAD,

-                    SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL)) {
+                    SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL,
+                    SERVICE_STOP_POST,
+                    SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) {

                 service_unwatch_main_pid(s);
                 s->main_command = NULL;
         }
                 service_unwatch_main_pid(s);
                 s->main_command = NULL;
         }


@@ -1504,7 +1507,8 @@ static void service_set_state(Service *s, ServiceState state) {
         if (!IN_SET(state,
                     SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST,
                     SERVICE_RELOAD,
         if (!IN_SET(state,
                     SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST,
                     SERVICE_RELOAD,

-                    SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, SERVICE_STOP_POST,
+                    SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL,
+                    SERVICE_STOP_POST,

                     SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) {
                 service_unwatch_control_pid(s);
                 s->control_command = NULL;
                     SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) {
                 service_unwatch_control_pid(s);
                 s->control_command = NULL;


@@ -1561,22 +1565,16 @@ static int service_coldplug(Unit *u) {
 
         if (s->deserialized_state != s->state) {
 
 
         if (s->deserialized_state != s->state) {
 

-                if (s->deserialized_state == SERVICE_START_PRE ||
-                    s->deserialized_state == SERVICE_START ||
-                    s->deserialized_state == SERVICE_START_POST ||
-                    s->deserialized_state == SERVICE_RELOAD ||
-                    s->deserialized_state == SERVICE_STOP ||
-                    s->deserialized_state == SERVICE_STOP_SIGTERM ||
-                    s->deserialized_state == SERVICE_STOP_SIGKILL ||
-                    s->deserialized_state == SERVICE_STOP_POST ||
-                    s->deserialized_state == SERVICE_FINAL_SIGTERM ||
-                    s->deserialized_state == SERVICE_FINAL_SIGKILL) {
+                if (IN_SET(s->deserialized_state,
+                           SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST,
+                           SERVICE_RELOAD,
+                           SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL,
+                           SERVICE_STOP_POST,
+                           SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) {

 
                         usec_t k;
 
 
                         usec_t k;
 

-                        k = s->deserialized_state == SERVICE_START_PRE || s->deserialized_state == SERVICE_START ||
-                                s->deserialized_state == SERVICE_START_POST || s->deserialized_state == SERVICE_RELOAD ?
-                                s->timeout_start_usec : s->timeout_stop_usec;
+                        k = IN_SET(s->deserialized_state, SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST, SERVICE_RELOAD) ? s->timeout_start_usec : s->timeout_stop_usec;

 
                         /* For the start/stop timeouts 0 means off */
                         if (k > 0) {
 
                         /* For the start/stop timeouts 0 means off */
                         if (k > 0) {


@@ -1594,38 +1592,30 @@ static int service_coldplug(Unit *u) {
                                 return r;
                 }
 
                                 return r;
                 }
 

-                if ((s->deserialized_state == SERVICE_START &&
-                     (s->type == SERVICE_FORKING ||
-                      s->type == SERVICE_DBUS ||
-                      s->type == SERVICE_ONESHOT ||
-                      s->type == SERVICE_NOTIFY)) ||
-                    s->deserialized_state == SERVICE_START_POST ||
-                    s->deserialized_state == SERVICE_RUNNING ||
-                    s->deserialized_state == SERVICE_RELOAD ||
-                    s->deserialized_state == SERVICE_STOP ||
-                    s->deserialized_state == SERVICE_STOP_SIGTERM ||
-                    s->deserialized_state == SERVICE_STOP_SIGKILL)
-                        if (s->main_pid > 0) {
-                                r = unit_watch_pid(UNIT(s), s->main_pid);
-                                if (r < 0)
-                                        return r;
-                        }
+                if (pid_valid(s->main_pid) &&
+                    ((s->deserialized_state == SERVICE_START && IN_SET(s->type, SERVICE_FORKING, SERVICE_DBUS, SERVICE_ONESHOT, SERVICE_NOTIFY)) ||
+                     IN_SET(s->deserialized_state,
+                            SERVICE_START, SERVICE_START_POST,
+                            SERVICE_RUNNING, SERVICE_RELOAD,
+                            SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL,
+                            SERVICE_STOP_POST,
+                            SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL))) {
+                        r = unit_watch_pid(UNIT(s), s->main_pid);
+                        if (r < 0)
+                                return r;
+                }

 
 

-                if (s->deserialized_state == SERVICE_START_PRE ||
-                    s->deserialized_state == SERVICE_START ||
-                    s->deserialized_state == SERVICE_START_POST ||
-                    s->deserialized_state == SERVICE_RELOAD ||
-                    s->deserialized_state == SERVICE_STOP ||
-                    s->deserialized_state == SERVICE_STOP_SIGTERM ||
-                    s->deserialized_state == SERVICE_STOP_SIGKILL ||
-                    s->deserialized_state == SERVICE_STOP_POST ||
-                    s->deserialized_state == SERVICE_FINAL_SIGTERM ||
-                    s->deserialized_state == SERVICE_FINAL_SIGKILL)
-                        if (s->control_pid > 0) {
-                                r = unit_watch_pid(UNIT(s), s->control_pid);
-                                if (r < 0)
-                                        return r;
-                        }
+                if (pid_valid(s->control_pid) &&
+                    IN_SET(s->deserialized_state,
+                           SERVICE_START_PRE, SERVICE_START, SERVICE_START_POST,
+                           SERVICE_RELOAD,
+                           SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL,
+                           SERVICE_STOP_POST,
+                           SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL)) {
+                        r = unit_watch_pid(UNIT(s), s->control_pid);
+                        if (r < 0)
+                                return r;
+                }

 
                 if (IN_SET(s->deserialized_state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD))
                         service_start_watchdog(s);
 
                 if (IN_SET(s->deserialized_state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD))
                         service_start_watchdog(s);


@@ -3022,6 +3012,14 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                                 /* If there is still a control process, wait for that first */
                                 break;
 
                                 /* If there is still a control process, wait for that first */
                                 break;
 

+                        case SERVICE_STOP_POST:
+                        case SERVICE_FINAL_SIGTERM:
+                        case SERVICE_FINAL_SIGKILL:
+
+                                if (!control_pid_good(s))
+                                        service_enter_dead(s, f, true);
+                                break;
+

                         default:
                                 assert_not_reached("Uh, main process died at wrong time.");
                         }
                         default:
                                 assert_not_reached("Uh, main process died at wrong time.");
                         }


@@ -3163,7 +3161,8 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                         case SERVICE_STOP_POST:
                         case SERVICE_FINAL_SIGTERM:
                         case SERVICE_FINAL_SIGKILL:
                         case SERVICE_STOP_POST:
                         case SERVICE_FINAL_SIGTERM:
                         case SERVICE_FINAL_SIGKILL:

-                                service_enter_dead(s, f, true);
+                                if (main_pid_good(s) <= 0)
+                                        service_enter_dead(s, f, true);

                                 break;
 
                         default:
                                 break;
 
                         default:


@@ -3327,6 +3326,7 @@ static void service_notify_cgroup_empty_event(Unit *u) {
 
                 break;
 
 
                 break;
 

+        case SERVICE_STOP_POST:

         case SERVICE_FINAL_SIGTERM:
         case SERVICE_FINAL_SIGKILL:
                 if (main_pid_good(s) <= 0 && !control_pid_good(s))
         case SERVICE_FINAL_SIGTERM:
         case SERVICE_FINAL_SIGKILL:
                 if (main_pid_good(s) <= 0 && !control_pid_good(s))

diff --git a/src/shared/util.c b/src/shared/util.c
index 2cf9fbe53e6f97c9b48e96f458d192741b185e10..f59897105a6354e1cea1c7261517e2a556c660ce 100644 (file)
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -6088,3 +6088,13 @@ int namespace_enter(int pidns_fd, int mntns_fd, int root_fd) {
 
         return 0;
 }
 
         return 0;
 }

+
+bool pid_valid(pid_t pid) {
+        if (pid <= 0)
+                return false;
+
+        if (kill(pid, 0) >= 0)
+                return true;
+
+        return errno != ESRCH;
+}

diff --git a/src/shared/util.h b/src/shared/util.h
index ddb35181abbdcca5d92e84177cea0e6c2ad50d70..3e0a6d5c1cd15a19f1af7ec8d69de649940f5b61 100644 (file)
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -804,3 +804,5 @@ int container_get_leader(const char *machine, pid_t *pid);
 
 int namespace_open(pid_t pid, int *pidns_fd, int *mntns_fd, int *root_fd);
 int namespace_enter(int pidns_fd, int mntns_fd, int root_fd);
 
 int namespace_open(pid_t pid, int *pidns_fd, int *mntns_fd, int *root_fd);
 int namespace_enter(int pidns_fd, int mntns_fd, int root_fd);

+
+bool pid_valid(pid_t pid);