process-spec: add another flag FORK_WAIT to safe_fork()

This new flag will cause safe_fork() to wait for the forked off child
before returning. This allows us to unify a number of cases where we
immediately wait on the forked off child, witout running any code in the
parent after the fork, and without direct interest in the precise exit
status of the process, except recgonizing EXIT_SUCCESS vs everything
else.

diff --git a/src/basic/calendarspec.c b/src/basic/calendarspec.c
index dbd7fb1d7643eb9ab479626fc41f3d6818da44aa..41dceb065c2c0828dfc373a7e90158b17f60b9ee 100644 (file)
--- a/src/basic/calendarspec.c
+++ b/src/basic/calendarspec.c
@@ -1349,7 +1349,6 @@ typedef struct SpecNextResult {
 
 int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next) {
         SpecNextResult *shared, tmp;
-        pid_t pid;
         int r;
 
         if (isempty(spec->timezone))
@@ -1359,7 +1358,7 @@ int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next)
         if (shared == MAP_FAILED)
                 return negative_errno();
 
-        r = safe_fork("(sd-calendar)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG, &pid);
+        r = safe_fork("(sd-calendar)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_WAIT, NULL);
         if (r < 0) {
                 (void) munmap(shared, sizeof *shared);
                 return r;
@@ -1377,14 +1376,8 @@ int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next)
                 _exit(EXIT_SUCCESS);
         }
 
-        r = wait_for_terminate(pid, NULL);
-        if (r < 0) {
-                (void) munmap(shared, sizeof *shared);
-                return r;
-        }
-
         tmp = *shared;
-        if (munmap(shared, sizeof *shared) != 0)
+        if (munmap(shared, sizeof *shared) < 0)
                 return negative_errno();
 
         if (tmp.return_value == 0)

diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c
index 95194e5f0550b4a5c238f2260747f384dca3e1b1..a97295a50918a8962cdd4fae6db75f2b44e56d52 100644 (file)
--- a/src/basic/exec-util.c
+++ b/src/basic/exec-util.c
@@ -189,10 +189,9 @@ int execute_directories(
                 void* const callback_args[_STDOUT_CONSUME_MAX],
                 char *argv[]) {
 
-        pid_t executor_pid;
-        char *name;
         char **dirs = (char**) directories;
         _cleanup_close_ int fd = -1;
+        char *name;
         int r;
 
         assert(!strv_isempty(dirs));
@@ -215,7 +214,7 @@ int execute_directories(
          * them to finish. Optionally a timeout is applied. If a file with the same name
          * exists in more than one directory, the earliest one wins. */
 
-        r = safe_fork("(sd-executor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &executor_pid);
+        r = safe_fork("(sd-executor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL);
         if (r < 0)
                 return r;
         if (r == 0) {
@@ -223,12 +222,6 @@ int execute_directories(
                 _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS);
         }
 
-        r = wait_for_terminate_and_check(name, executor_pid, WAIT_LOG);
-        if (r < 0)
-                return r;
-        if (r > 0) /* non-zero return code from child */
-                return -EREMOTEIO;
-
         if (!callbacks)
                 return 0;
 

diff --git a/src/basic/process-util.c b/src/basic/process-util.c
index 9ca187a95447e152b85897284e981057bc0f8d5c..69f1d1e7b4e09f7eac0e7cde247fc083cf446217 100644 (file)
--- a/src/basic/process-util.c
+++ b/src/basic/process-util.c
@@ -1161,7 +1161,7 @@ int safe_fork_full(
                 pid_t *ret_pid) {
 
         pid_t original_pid, pid;
-        sigset_t saved_ss;
+        sigset_t saved_ss, ss;
         bool block_signals;
         int prio, r;
 
@@ -1172,20 +1172,33 @@ int safe_fork_full(
 
         original_pid = getpid_cached();
 
-        block_signals = flags & (FORK_RESET_SIGNALS|FORK_DEATHSIG);
+        if (flags & (FORK_RESET_SIGNALS|FORK_DEATHSIG)) {
 
-        if (block_signals) {
-                sigset_t ss;
+                /* We temporarily block all signals, so that the new child has them blocked initially. This way, we can
+                 * be sure that SIGTERMs are not lost we might send to the child. */
 
-                /* We temporarily block all signals, so that the new child has them blocked initially. This way, we can be sure
-                 * that SIGTERMs are not lost we might send to the child. */
                 if (sigfillset(&ss) < 0)
                         return log_full_errno(prio, errno, "Failed to reset signal set: %m");
 
-                if (sigprocmask(SIG_SETMASK, &ss, &saved_ss) < 0)
-                        return log_full_errno(prio, errno, "Failed to reset signal mask: %m");
+                block_signals = true;
+
+        } else if (flags & FORK_WAIT) {
+
+                /* Let's block SIGCHLD at least, so that we can safely watch for the child process */
+
+                if (sigemptyset(&ss) < 0)
+                        return log_full_errno(prio, errno, "Failed to clear signal set: %m");
+
+                if (sigaddset(&ss, SIGCHLD) < 0)
+                        return log_full_errno(prio, errno, "Failed to add SIGCHLD to signal set: %m");
+
+                block_signals = true;
         }
 
+        if (block_signals)
+                if (sigprocmask(SIG_SETMASK, &ss, &saved_ss) < 0)
+                        return log_full_errno(prio, errno, "Failed to set signal mask: %m");
+
         pid = fork();
         if (pid < 0) {
                 r = -errno;
@@ -1198,11 +1211,19 @@ int safe_fork_full(
         if (pid > 0) {
                 /* We are in the parent process */
 
+                log_debug("Successfully forked off '%s' as PID " PID_FMT ".", strna(name), pid);
+
+                if (flags & FORK_WAIT) {
+                        r = wait_for_terminate_and_check(name, pid, (flags & FORK_LOG ? WAIT_LOG : 0));
+                        if (r < 0)
+                                return r;
+                        if (r != EXIT_SUCCESS) /* exit status > 0 should be treated as failure, too */
+                                return -EPROTO;
+                }
+
                 if (block_signals) /* undo what we did above */
                         (void) sigprocmask(SIG_SETMASK, &saved_ss, NULL);
 
-                log_debug("Sucessfully forked off '%s' as PID " PID_FMT ".", strna(name), pid);
-
                 if (ret_pid)
                         *ret_pid = pid;
 

diff --git a/src/basic/process-util.h b/src/basic/process-util.h
index 1afb860264122f05df82add41a2296414be3453e..ba247a089d94562054d28e83873a91e6fde28fb2 100644 (file)
--- a/src/basic/process-util.h
+++ b/src/basic/process-util.h
@@ -167,6 +167,7 @@ typedef enum ForkFlags {
         FORK_NULL_STDIO    = 1U << 3,
         FORK_REOPEN_LOG    = 1U << 4,
         FORK_LOG           = 1U << 5,
+        FORK_WAIT          = 1U << 6,
 } ForkFlags;
 
 int safe_fork_full(const char *name, const int except_fds[], size_t n_except_fds, ForkFlags flags, pid_t *ret_pid);

diff --git a/src/basic/time-util.c b/src/basic/time-util.c
index 95358f8e9fc4b7858223d23a0c8f48e6b0e8326a..cbb5285018f0667f6667d64e6aa6b41c4d490d49 100644 (file)
--- a/src/basic/time-util.c
+++ b/src/basic/time-util.c
@@ -886,7 +886,6 @@ typedef struct ParseTimestampResult {
 int parse_timestamp(const char *t, usec_t *usec) {
         char *last_space, *tz = NULL;
         ParseTimestampResult *shared, tmp;
-        pid_t pid;
         int r;
 
         last_space = strrchr(t, ' ');
@@ -900,7 +899,7 @@ int parse_timestamp(const char *t, usec_t *usec) {
         if (shared == MAP_FAILED)
                 return negative_errno();
 
-        r = safe_fork("(sd-timestamp)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG, &pid);
+        r = safe_fork("(sd-timestamp)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_WAIT, NULL);
         if (r < 0) {
                 (void) munmap(shared, sizeof *shared);
                 return r;
@@ -928,12 +927,6 @@ int parse_timestamp(const char *t, usec_t *usec) {
                 _exit(EXIT_SUCCESS);
         }
 
-        r = wait_for_terminate(pid, NULL);
-        if (r < 0) {
-                (void) munmap(shared, sizeof *shared);
-                return r;
-        }
-
         tmp = *shared;
         if (munmap(shared, sizeof *shared) != 0)
                 return negative_errno();

diff --git a/src/core/shutdown.c b/src/core/shutdown.c
index 1be0f8b603fbd5a6116aa90c7c0565de86a5f8e3..ffab4de1013493cfc3656367939d3db9317cde52 100644 (file)
--- a/src/core/shutdown.c
+++ b/src/core/shutdown.c
@@ -483,13 +483,10 @@ int main(int argc, char *argv[]) {
 
                 if (!in_container) {
                         /* We cheat and exec kexec to avoid doing all its work */
-                        pid_t pid;
-
                         log_info("Rebooting with kexec.");
 
-                        r = safe_fork("(sd-kexec)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG, &pid);
+                        r = safe_fork("(sd-kexec)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG|FORK_WAIT, NULL);
                         if (r == 0) {
-
                                 const char * const args[] = {
                                         KEXEC, "-e", NULL
                                 };
@@ -500,7 +497,7 @@ int main(int argc, char *argv[]) {
                                 _exit(EXIT_FAILURE);
                         }
 
-                        (void) wait_for_terminate_and_check("kexec", pid, WAIT_LOG);
+                        /* If we are still running, then the kexec can't have worked, let's fall through */
                 }
 
                 cmd = RB_AUTOBOOT;

diff --git a/src/quotacheck/quotacheck.c b/src/quotacheck/quotacheck.c
index c40c7d5d07942e0f9791b743f8578b24d60c767b..1bf718e4f61b800ac6130b10fea8b6be63922066 100644 (file)
--- a/src/quotacheck/quotacheck.c
+++ b/src/quotacheck/quotacheck.c
@@ -71,14 +71,6 @@ static void test_files(void) {
 }
 
 int main(int argc, char *argv[]) {
-
-        static const char * const cmdline[] = {
-                QUOTACHECK,
-                "-anug",
-                NULL
-        };
-
-        pid_t pid;
         int r;
 
         if (argc > 1) {
@@ -106,10 +98,15 @@ int main(int argc, char *argv[]) {
                         return EXIT_SUCCESS;
         }
 
-        r = safe_fork("(quotacheck)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid);
+        r = safe_fork("(quotacheck)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL);
         if (r < 0)
                 goto finish;
         if (r == 0) {
+                static const char * const cmdline[] = {
+                        QUOTACHECK,
+                        "-anug",
+                        NULL
+                };
 
                 /* Child */
 
@@ -117,8 +114,6 @@ int main(int argc, char *argv[]) {
                 _exit(EXIT_FAILURE); /* Operational error */
         }
 
-        r = wait_for_terminate_and_check("quotacheck", pid, WAIT_LOG);
-
 finish:
         return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
 }

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index fc921d1dd7dabb416fa75fa3e86c8d634d86b65b..741144b6cbb26715b70b2120b3dc940f74167349 100644 (file)
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -6987,12 +6987,11 @@ static int unit_file_create_copy(
 }
 
 static int run_editor(char **paths) {
-        pid_t pid;
         int r;
 
         assert(paths);
 
-        r = safe_fork("(editor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid);
+        r = safe_fork("(editor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL);
         if (r < 0)
                 return r;
         if (r == 0) {
@@ -7057,10 +7056,6 @@ static int run_editor(char **paths) {
                 _exit(EXIT_FAILURE);
         }
 
-        r = wait_for_terminate_and_check("editor", pid, WAIT_LOG);
-        if (r < 0)
-                return r;
-
         return 0;
 }