basic/exec-util: add support for synchronous (ordered) execution

The output of processes can be gathered, and passed back to the callee.
(This commit just implements the basic functionality and tests.)

After the preparation in previous commits, the change in functionality is
relatively simple. For coding convenience, alarm is prepared *before* any
children are executed, and not before. This shouldn't matter usually, since
just forking of the children should be pretty quick. One could also argue that
this is more correct, because we will also catch the case when (for whatever
reason), forking itself is slow.

Three callback functions and three levels of serialization are used:
- from individual generator processes to the generator forker
- from the forker back to the main process
- deserialization in the main process

v2:
- replace an structure with an indexed array of callbacks

6 files changed, 349 insertions, 47 deletions

diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c
index 46eafc7b90..d69039c489 100644
--- a/src/basic/exec-util.c
+++ b/src/basic/exec-util.c
@@ -22,10 +22,14 @@
 #include <sys/prctl.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <stdio.h>
 
 #include "alloc-util.h"
 #include "conf-files.h"
+#include "env-util.h"
 #include "exec-util.h"
+#include "fd-util.h"
+#include "fileio.h"
 #include "hashmap.h"
 #include "macro.h"
 #include "process-util.h"
@@ -34,12 +38,14 @@
 #include "stat-util.h"
 #include "string-util.h"
 #include "strv.h"
+#include "terminal-util.h"
 #include "util.h"
 
 /* Put this test here for a lack of better place */
 assert_cc(EAGAIN == EWOULDBLOCK);
 
-static int do_spawn(const char *path, char *argv[], pid_t *pid) {
+static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid) {
+
         pid_t _pid;
 
         if (null_or_empty_path(path)) {
@@ -55,6 +61,15 @@ static int do_spawn(const char *path, char *argv[], pid_t *pid) {
 
                 assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0);
 
+                if (stdout_fd >= 0) {
+                        /* If the fd happens to be in the right place, go along with that */
+                        if (stdout_fd != STDOUT_FILENO &&
+                            dup2(stdout_fd, STDOUT_FILENO) < 0)
+                                return -errno;
+
+                        fd_cloexec(STDOUT_FILENO, false);
+                }
+
                 if (!argv) {
                         _argv[0] = (char*) path;
                         _argv[1] = NULL;
@@ -72,14 +87,24 @@ static int do_spawn(const char *path, char *argv[], pid_t *pid) {
         return 1;
 }
 
-static int do_execute(char **directories, usec_t timeout, char *argv[]) {
+static int do_execute(
+                char **directories,
+                usec_t timeout,
+                gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX],
+                void* const callback_args[_STDOUT_CONSUME_MAX],
+                int output_fd,
+                char *argv[]) {
+
         _cleanup_hashmap_free_free_ Hashmap *pids = NULL;
         _cleanup_strv_free_ char **paths = NULL;
         char **path;
         int r;
 
-        /* We fork this all off from a child process so that we can
-         * somewhat cleanly make use of SIGALRM to set a time limit */
+        /* We fork this all off from a child process so that we can somewhat cleanly make
+         * use of SIGALRM to set a time limit.
+         *
+         * If callbacks is nonnull, execution is serial. Otherwise, we default to parallel.
+         */
 
         (void) reset_all_signal_handlers();
         (void) reset_signal_mask();
@@ -90,35 +115,62 @@ static int do_execute(char **directories, usec_t timeout, char *argv[]) {
         if (r < 0)
                 return r;
 
-        pids = hashmap_new(NULL);
-        if (!pids)
-                return log_oom();
+        if (!callbacks) {
+                pids = hashmap_new(NULL);
+                if (!pids)
+                        return log_oom();
+        }
+
+        /* Abort execution of this process after the timout. We simply rely on SIGALRM as
+         * default action terminating the process, and turn on alarm(). */
+
+        if (timeout != USEC_INFINITY)
+                alarm((timeout + USEC_PER_SEC - 1) / USEC_PER_SEC);
 
         STRV_FOREACH(path, paths) {
                 _cleanup_free_ char *t = NULL;
+                _cleanup_close_ int fd = -1;
                 pid_t pid;
 
                 t = strdup(*path);
                 if (!t)
                         return log_oom();
 
-                r = do_spawn(t, argv, &pid);
+                if (callbacks) {
+                        fd = open_serialization_fd(basename(*path));
+                        if (fd < 0)
+                                return log_error_errno(fd, "Failed to open serialization file: %m");
+                }
+
+                r = do_spawn(t, argv, fd, &pid);
                 if (r <= 0)
                         continue;
 
-                r = hashmap_put(pids, PID_TO_PTR(pid), t);
-                if (r < 0)
-                        return log_oom();
-
-                t = NULL;
+                if (pids) {
+                        r = hashmap_put(pids, PID_TO_PTR(pid), t);
+                        if (r < 0)
+                                return log_oom();
+                        t = NULL;
+                } else {
+                        r = wait_for_terminate_and_warn(t, pid, true);
+                        if (r < 0)
+                                continue;
+
+                        if (lseek(fd, 0, SEEK_SET) < 0)
+                                return log_error_errno(errno, "Failed to seek on serialization fd: %m");
+
+                        r = callbacks[STDOUT_GENERATE](fd, callback_args[STDOUT_GENERATE]);
+                        fd = -1;
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to process output from %s: %m", *path);
+                }
         }
 
-        /* Abort execution of this process after the timout. We simply
-         * rely on SIGALRM as default action terminating the process,
-         * and turn on alarm(). */
-
-        if (timeout != USEC_INFINITY)
-                alarm((timeout + USEC_PER_SEC - 1) / USEC_PER_SEC);
+        if (callbacks) {
+                r = callbacks[STDOUT_COLLECT](output_fd, callback_args[STDOUT_COLLECT]);
+                if (r < 0)
+                        return log_error_errno(r, "Callback two failed: %m");
+        }
 
         while (!hashmap_isempty(pids)) {
                 _cleanup_free_ char *t = NULL;
@@ -136,31 +188,66 @@ static int do_execute(char **directories, usec_t timeout, char *argv[]) {
         return 0;
 }
 
-void execute_directories(const char* const* directories, usec_t timeout, char *argv[]) {
+int execute_directories(
+                const char* const* directories,
+                usec_t timeout,
+                gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX],
+                void* const callback_args[_STDOUT_CONSUME_MAX],
+                char *argv[]) {
+
         pid_t executor_pid;
-        int r;
         char *name;
         char **dirs = (char**) directories;
+        _cleanup_close_ int fd = -1;
+        int r;
 
         assert(!strv_isempty(dirs));
 
         name = basename(dirs[0]);
         assert(!isempty(name));
 
-        /* Executes all binaries in the directories in parallel and waits
-         * for 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. */
+        if (callbacks) {
+                assert(callback_args);
+                assert(callbacks[STDOUT_GENERATE]);
+                assert(callbacks[STDOUT_COLLECT]);
+                assert(callbacks[STDOUT_CONSUME]);
+
+                fd = open_serialization_fd(name);
+                if (fd < 0)
+                        return log_error_errno(fd, "Failed to open serialization file: %m");
+        }
+
+        /* Executes all binaries in the directories serially or in parallel and waits for
+         * 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. */
 
         executor_pid = fork();
-        if (executor_pid < 0) {
-                log_error_errno(errno, "Failed to fork: %m");
-                return;
+        if (executor_pid < 0)
+                return log_error_errno(errno, "Failed to fork: %m");
 
-        } else if (executor_pid == 0) {
-                r = do_execute(dirs, timeout, argv);
+        if (executor_pid == 0) {
+                r = do_execute(dirs, timeout, callbacks, callback_args, fd, argv);
                 _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS);
         }
 
-        wait_for_terminate_and_warn(name, executor_pid, true);
+        r = wait_for_terminate_and_warn(name, executor_pid, true);
+        if (r < 0)
+                return log_error_errno(r, "Execution failed: %m");
+        if (r > 0) {
+                /* non-zero return code from child */
+                log_error("Forker process failed.");
+                return -EREMOTEIO;
+        }
+
+        if (!callbacks)
+                return 0;
+
+        if (lseek(fd, 0, SEEK_SET) < 0)
+                return log_error_errno(errno, "Failed to rewind serialization fd: %m");
+
+        r = callbacks[STDOUT_CONSUME](fd, callback_args[STDOUT_CONSUME]);
+        fd = -1;
+        if (r < 0)
+                return log_error_errno(r, "Failed to parse returned data: %m");
+        return 0;
 }
diff --git a/src/basic/exec-util.h b/src/basic/exec-util.h
index 9f8daa9fc8..2c58e4bd5c 100644
--- a/src/basic/exec-util.h
+++ b/src/basic/exec-util.h
@@ -17,6 +17,22 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
+#include <stdbool.h>
+
 #include "time-util.h"
 
-void execute_directories(const char* const* directories, usec_t timeout, char *argv[]);
+typedef int (*gather_stdout_callback_t) (int fd, void *arg);
+
+enum {
+        STDOUT_GENERATE,   /* from generators to helper process */
+        STDOUT_COLLECT,    /* from helper process to main process */
+        STDOUT_CONSUME,    /* process data in main process */
+        _STDOUT_CONSUME_MAX,
+};
+
+int execute_directories(
+                const char* const* directories,
+                usec_t timeout,
+                gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX],
+                void* const callback_args[_STDOUT_CONSUME_MAX],
+                char *argv[]);
diff --git a/src/core/manager.c b/src/core/manager.c
index d432512a59..73ac7499bd 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3047,7 +3047,8 @@ static int manager_run_generators(Manager *m) {
         argv[4] = NULL;
 
         RUN_WITH_UMASK(0022)
-                execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC, (char**) argv);
+                execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC,
+                                    NULL, NULL, (char**) argv);
 
 finish:
         lookup_paths_trim_generator(&m->lookup_paths);
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
index 56a035e234..a2309b7726 100644
--- a/src/core/shutdown.c
+++ b/src/core/shutdown.c
@@ -322,7 +322,7 @@ int main(int argc, char *argv[]) {
         arguments[0] = NULL;
         arguments[1] = arg_verb;
         arguments[2] = NULL;
-        execute_directories(dirs, DEFAULT_TIMEOUT_USEC, arguments);
+        execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments);
 
         if (!in_container && !in_initrd() &&
             access("/run/initramfs/shutdown", X_OK) == 0) {
diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c
index b0f992fc9c..a6978b6273 100644
--- a/src/sleep/sleep.c
+++ b/src/sleep/sleep.c
@@ -107,7 +107,7 @@ static int execute(char **modes, char **states) {
         if (r < 0)
                 return r;
 
-        execute_directories(dirs, DEFAULT_TIMEOUT_USEC, arguments);
+        execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments);
 
         log_struct(LOG_INFO,
                    LOG_MESSAGE_ID(SD_MESSAGE_SLEEP_START),
@@ -126,7 +126,7 @@ static int execute(char **modes, char **states) {
                    NULL);
 
         arguments[1] = (char*) "post";
-        execute_directories(dirs, DEFAULT_TIMEOUT_USEC, arguments);
+        execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments);
 
         return r;
 }
diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c
index 26533f0bf6..41cbef74b1 100644
--- a/src/test/test-exec-util.c
+++ b/src/test/test-exec-util.c
@@ -24,21 +24,59 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
+#include "alloc-util.h"
+#include "copy.h"
 #include "def.h"
 #include "exec-util.h"
+#include "fd-util.h"
 #include "fileio.h"
 #include "fs-util.h"
 #include "log.h"
 #include "macro.h"
 #include "rm-rf.h"
 #include "string-util.h"
+#include "strv.h"
 
-static void test_execute_directory(void) {
-        char template_lo[] = "/tmp/test-readlink_and_make_absolute-lo.XXXXXXX";
-        char template_hi[] = "/tmp/test-readlink_and_make_absolute-hi.XXXXXXX";
+static int here = 0, here2 = 0, here3 = 0;
+void *ignore_stdout_args[] = {&here, &here2, &here3};
+
+/* noop handlers, just check that arguments are passed correctly */
+static int ignore_stdout_func(int fd, void *arg) {
+        assert(fd >= 0);
+        assert(arg == &here);
+        safe_close(fd);
+
+        return 0;
+}
+static int ignore_stdout_func2(int fd, void *arg) {
+        assert(fd >= 0);
+        assert(arg == &here2);
+        safe_close(fd);
+
+        return 0;
+}
+static int ignore_stdout_func3(int fd, void *arg) {
+        assert(fd >= 0);
+        assert(arg == &here3);
+        safe_close(fd);
+
+        return 0;
+}
+
+static const gather_stdout_callback_t ignore_stdout[] = {
+        ignore_stdout_func,
+        ignore_stdout_func2,
+        ignore_stdout_func3,
+};
+
+static void test_execute_directory(bool gather_stdout) {
+        char template_lo[] = "/tmp/test-exec-util.XXXXXXX";
+        char template_hi[] = "/tmp/test-exec-util.XXXXXXX";
         const char * dirs[] = {template_hi, template_lo, NULL};
         const char *name, *name2, *name3, *overridden, *override, *masked, *mask;
 
+        log_info("/* %s (%s) */", __func__, gather_stdout ? "gathering stdout" : "asynchronous");
+
         assert_se(mkdtemp(template_lo));
         assert_se(mkdtemp(template_hi));
 
@@ -50,20 +88,34 @@ static void test_execute_directory(void) {
         masked = strjoina(template_lo, "/masked");
         mask = strjoina(template_hi, "/masked");
 
-        assert_se(write_string_file(name, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works", WRITE_STRING_FILE_CREATE) == 0);
-        assert_se(write_string_file(name2, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works2", WRITE_STRING_FILE_CREATE) == 0);
-        assert_se(write_string_file(overridden, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed", WRITE_STRING_FILE_CREATE) == 0);
-        assert_se(write_string_file(override, "#!/bin/sh\necho 'Executing '$0", WRITE_STRING_FILE_CREATE) == 0);
-        assert_se(write_string_file(masked, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed", WRITE_STRING_FILE_CREATE) == 0);
+        assert_se(write_string_file(name,
+                                    "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works",
+                                    WRITE_STRING_FILE_CREATE) == 0);
+        assert_se(write_string_file(name2,
+                                    "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works2",
+                                    WRITE_STRING_FILE_CREATE) == 0);
+        assert_se(write_string_file(overridden,
+                                    "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed",
+                                    WRITE_STRING_FILE_CREATE) == 0);
+        assert_se(write_string_file(override,
+                                    "#!/bin/sh\necho 'Executing '$0",
+                                    WRITE_STRING_FILE_CREATE) == 0);
+        assert_se(write_string_file(masked,
+                                    "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed",
+                                    WRITE_STRING_FILE_CREATE) == 0);
         assert_se(symlink("/dev/null", mask) == 0);
+        assert_se(touch(name3) >= 0);
+
         assert_se(chmod(name, 0755) == 0);
         assert_se(chmod(name2, 0755) == 0);
         assert_se(chmod(overridden, 0755) == 0);
         assert_se(chmod(override, 0755) == 0);
         assert_se(chmod(masked, 0755) == 0);
-        assert_se(touch(name3) >= 0);
 
-        execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL);
+        if (gather_stdout)
+                execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL);
+        else
+                execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, NULL);
 
         assert_se(chdir(template_lo) == 0);
         assert_se(access("it_works", F_OK) >= 0);
@@ -77,11 +129,157 @@ static void test_execute_directory(void) {
         (void) rm_rf(template_hi, REMOVE_ROOT|REMOVE_PHYSICAL);
 }
 
+static void test_execution_order(void) {
+        char template_lo[] = "/tmp/test-exec-util-lo.XXXXXXX";
+        char template_hi[] = "/tmp/test-exec-util-hi.XXXXXXX";
+        const char *dirs[] = {template_hi, template_lo, NULL};
+        const char *name, *name2, *name3, *overridden, *override, *masked, *mask;
+        const char *output, *t;
+        _cleanup_free_ char *contents = NULL;
+
+        assert_se(mkdtemp(template_lo));
+        assert_se(mkdtemp(template_hi));
+
+        output = strjoina(template_hi, "/output");
+
+        log_info("/* %s >>%s */", __func__, output);
+
+        /* write files in "random" order */
+        name2 = strjoina(template_lo, "/90-bar");
+        name = strjoina(template_hi, "/80-foo");
+        name3 = strjoina(template_lo, "/last");
+        overridden = strjoina(template_lo, "/30-override");
+        override = strjoina(template_hi, "/30-override");
+        masked = strjoina(template_lo, "/10-masked");
+        mask = strjoina(template_hi, "/10-masked");
+
+        t = strjoina("#!/bin/sh\necho $(basename $0) >>", output);
+        assert_se(write_string_file(name, t, WRITE_STRING_FILE_CREATE) == 0);
+
+        t = strjoina("#!/bin/sh\necho $(basename $0) >>", output);
+        assert_se(write_string_file(name2, t, WRITE_STRING_FILE_CREATE) == 0);
+
+        t = strjoina("#!/bin/sh\necho $(basename $0) >>", output);
+        assert_se(write_string_file(name3, t, WRITE_STRING_FILE_CREATE) == 0);
+
+        t = strjoina("#!/bin/sh\necho OVERRIDDEN >>", output);
+        assert_se(write_string_file(overridden, t, WRITE_STRING_FILE_CREATE) == 0);
+
+        t = strjoina("#!/bin/sh\necho $(basename $0) >>", output);
+        assert_se(write_string_file(override, t, WRITE_STRING_FILE_CREATE) == 0);
+
+        t = strjoina("#!/bin/sh\necho MASKED >>", output);
+        assert_se(write_string_file(masked, t, WRITE_STRING_FILE_CREATE) == 0);
+
+        assert_se(symlink("/dev/null", mask) == 0);
+
+        assert_se(chmod(name, 0755) == 0);
+        assert_se(chmod(name2, 0755) == 0);
+        assert_se(chmod(name3, 0755) == 0);
+        assert_se(chmod(overridden, 0755) == 0);
+        assert_se(chmod(override, 0755) == 0);
+        assert_se(chmod(masked, 0755) == 0);
+
+        execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL);
+
+        assert_se(read_full_file(output, &contents, NULL) >= 0);
+        assert_se(streq(contents, "30-override\n80-foo\n90-bar\nlast\n"));
+
+        (void) rm_rf(template_lo, REMOVE_ROOT|REMOVE_PHYSICAL);
+        (void) rm_rf(template_hi, REMOVE_ROOT|REMOVE_PHYSICAL);
+}
+
+static int gather_stdout_one(int fd, void *arg) {
+        char ***s = arg, *t;
+        char buf[128] = {};
+
+        assert_se(s);
+        assert_se(read(fd, buf, sizeof buf) >= 0);
+        safe_close(fd);
+
+        assert_se(t = strndup(buf, sizeof buf));
+        assert_se(strv_push(s, t) >= 0);
+
+        return 0;
+}
+static int gather_stdout_two(int fd, void *arg) {
+        char ***s = arg, **t;
+
+        STRV_FOREACH(t, *s)
+                assert_se(write(fd, *t, strlen(*t)) == (ssize_t) strlen(*t));
+        safe_close(fd);
+
+        return 0;
+}
+static int gather_stdout_three(int fd, void *arg) {
+        char **s = arg;
+        char buf[128] = {};
+
+        assert_se(read(fd, buf, sizeof buf - 1) > 0);
+        safe_close(fd);
+        assert_se(*s = strndup(buf, sizeof buf));
+
+        return 0;
+}
+
+const gather_stdout_callback_t const gather_stdout[] = {
+        gather_stdout_one,
+        gather_stdout_two,
+        gather_stdout_three,
+};
+
+
+static void test_stdout_gathering(void) {
+        char template[] = "/tmp/test-exec-util.XXXXXXX";
+        const char *dirs[] = {template, NULL};
+        const char *name, *name2, *name3;
+        int r;
+
+        char **tmp = NULL; /* this is only used in the forked process, no cleanup here */
+        _cleanup_free_ char *output = NULL;
+
+        void* args[] = {&tmp, &tmp, &output};
+
+        assert_se(mkdtemp(template));
+
+        log_info("/* %s */", __func__);
+
+        /* write files */
+        name = strjoina(template, "/10-foo");
+        name2 = strjoina(template, "/20-bar");
+        name3 = strjoina(template, "/30-last");
+
+        assert_se(write_string_file(name,
+                                    "#!/bin/sh\necho a\necho b\necho c\n",
+                                    WRITE_STRING_FILE_CREATE) == 0);
+        assert_se(write_string_file(name2,
+                                    "#!/bin/sh\necho d\n",
+                                    WRITE_STRING_FILE_CREATE) == 0);
+        assert_se(write_string_file(name3,
+                                    "#!/bin/sh\nsleep 1",
+                                    WRITE_STRING_FILE_CREATE) == 0);
+
+        assert_se(chmod(name, 0755) == 0);
+        assert_se(chmod(name2, 0755) == 0);
+        assert_se(chmod(name3, 0755) == 0);
+
+        r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_stdout, args, NULL);
+        assert_se(r >= 0);
+
+        log_info("got: %s", output);
+
+        assert_se(streq(output, "a\nb\nc\nd\n"));
+}
+
 int main(int argc, char *argv[]) {
+        log_set_max_level(LOG_DEBUG);
         log_parse_environment();
         log_open();
 
-        test_execute_directory();
+        test_execute_directory(true);
+        test_execute_directory(false);
+        test_execution_order();
+        test_stdout_gathering();
 
         return 0;
 }