shell: Rewrite client_exec as client_launch

This patch is a major rewrite of the client_exec which fixes a couple of
issues found by upstream.

Specifically this address the following two issues:

- Do not weston_log() after fork()
- Own the session for the launched client

These two issues were integrated into this single patch.

It makes use of previously added wrappers to handle custom environment
being passed to the (shell) client being executed.

(Based on the work from https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/954
and from https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/817)

Bug-AGL: SPEC-4509

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Change-Id: I29d4bcacba3671f67bb915bdb55a80b556e143ac

diff --git a/meson.build b/meson.build
index 0daf943..0958f06 100644 (file)
--- a/meson.build
+++ b/meson.build
@@ -171,6 +171,7 @@ srcs_agl_compositor = [
        'src/input.c',
        'shared/option-parser.c',
        'shared/os-compatibility.c',
+       'shared/process-util.c',
        agl_shell_server_protocol_h,
        agl_shell_desktop_server_protocol_h,
        agl_screenshooter_server_protocol_h,

diff --git a/src/compositor.c b/src/compositor.c
index 7540fe3..e03b860 100644 (file)
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1646,6 +1646,7 @@ int wet_main(int argc, char *argv[], const struct weston_testsuite_data *test_da
        wl_list_init(&ivi.split_pending_apps);
        wl_list_init(&ivi.remote_pending_apps);
        wl_list_init(&ivi.desktop_clients);
+       wl_list_init(&ivi.child_process_list);
 
        /* Prevent any clients we spawn getting our stdin */
        os_fd_set_cloexec(STDIN_FILENO);

diff --git a/src/ivi-compositor.h b/src/ivi-compositor.h
index bc5cc68..ebbcc17 100644 (file)
--- a/src/ivi-compositor.h
+++ b/src/ivi-compositor.h
@@ -112,6 +112,8 @@ struct ivi_compositor {
        struct weston_layer panel;
        struct weston_layer popup;
        struct weston_layer fullscreen;
+
+       struct wl_list child_process_list;
 };
 
 struct ivi_surface;

diff --git a/src/shell.c b/src/shell.c
index f7ec6a2..cb6e592 100644 (file)
--- a/src/shell.c
+++ b/src/shell.c
@@ -37,7 +37,10 @@
 #include <libweston/libweston.h>
 #include <libweston/config-parser.h>
 
+#include <weston/weston.h>
 #include "shared/os-compatibility.h"
+#include "shared/helpers.h"
+#include "shared/process-util.h"
 
 #include "agl-shell-server-protocol.h"
 #include "agl-shell-desktop-server-protocol.h"
@@ -812,82 +815,148 @@ ivi_shell_advertise_xdg_surfaces(struct ivi_compositor *ivi, struct wl_resource
        }
 }
 
-static void
-client_exec(const char *command, int fd)
+static struct wl_client *
+client_launch(struct weston_compositor *compositor,
+                    struct weston_process *proc,
+                    const char *path,
+                    weston_process_cleanup_func_t cleanup)
 {
-       sigset_t sig;
-       char s[32];
+       struct wl_client *client = NULL;
+       struct custom_env child_env;
+       struct fdstr wayland_socket;
+       const char *fail_cloexec = "Couldn't unset CLOEXEC on client socket";
+       const char *fail_seteuid = "Couldn't call seteuid";
+       char *fail_exec;
+       char * const *argp;
+       char * const *envp;
+       sigset_t allsigs;
+       pid_t pid;
+       bool ret;
+       struct ivi_compositor *ivi;
+       size_t written __attribute__((unused));
 
-       /* Don't give the child our signal mask */
-       sigfillset(&sig);
-       sigprocmask(SIG_UNBLOCK, &sig, NULL);
+       weston_log("launching '%s'\n", path);
+       str_printf(&fail_exec, "Error: Couldn't launch client '%s'\n", path);
 
-       /* Launch clients as the user; don't give them the wrong euid */
-       if (seteuid(getuid()) == -1) {
-               weston_log("seteuid failed: %s\n", strerror(errno));
-               return;
-       }
+       custom_env_init_from_environ(&child_env);
+       custom_env_add_from_exec_string(&child_env, path);
 
-       /* Duplicate fd to unset the CLOEXEC flag. We don't need to worry about
-        * clobbering fd, as we'll exit/exec either way.
-        */
-       fd = dup(fd);
-       if (fd == -1) {
-               weston_log("dup failed: %s\n", strerror(errno));
-               return;
+       if (os_socketpair_cloexec(AF_UNIX, SOCK_STREAM, 0,
+                                 wayland_socket.fds) < 0) {
+               weston_log("client_launch: "
+                          "socketpair failed while launching '%s': %s\n",
+                          path, strerror(errno));
+               custom_env_fini(&child_env);
+               return NULL;
        }
+       fdstr_update_str1(&wayland_socket);
+       custom_env_set_env_var(&child_env, "WAYLAND_SOCKET",
+                              wayland_socket.str1);
 
-       snprintf(s, sizeof s, "%d", fd);
-       setenv("WAYLAND_SOCKET", s, 1);
+       argp = custom_env_get_argp(&child_env);
+       envp = custom_env_get_envp(&child_env);
 
-       execl("/bin/sh", "/bin/sh", "-c", command, NULL);
-       weston_log("executing '%s' failed: %s", command, strerror(errno));
-}
+       pid = fork();
+       switch (pid) {
+       case 0:
+               /* Put the client in a new session so it won't catch signals
+                * intended for the parent. Sharing a session can be
+                * confusing when launching weston under gdb, as the ctrl-c
+                * intended for gdb will pass to the child, and weston
+                * will cleanly shut down when the child exits.
+                */
+               setsid();
+
+               /* do not give our signal mask to the new process */
+               sigfillset(&allsigs);
+               sigprocmask(SIG_UNBLOCK, &allsigs, NULL);
+
+               /* Launch clients as the user. Do not launch clients with wrong euid. */
+               if (seteuid(getuid()) == -1) {
+                       written = write(STDERR_FILENO, fail_seteuid, strlen(fail_seteuid));
+                       _exit(EXIT_FAILURE);
+               }
 
-static struct wl_client *
-launch_shell_client(struct ivi_compositor *ivi, const char *command)
-{
-       struct wl_client *client;
-       int sock[2];
-       pid_t pid;
+               ret = fdstr_clear_cloexec_fd1(&wayland_socket);
+               if (!ret) {
+                       written = write(STDERR_FILENO, fail_cloexec, strlen(fail_cloexec));
+                       _exit(EXIT_FAILURE);
+               }
 
-       weston_log("launching' %s'\n", command);
+               execve(argp[0], argp, envp);
 
-       if (os_socketpair_cloexec(AF_UNIX, SOCK_STREAM, 0, sock) < 0) {
-               weston_log("socketpair failed while launching '%s': %s\n",
-                          command, strerror(errno));
-               return NULL;
-       }
+               if (fail_exec)
+                       written = write(STDERR_FILENO, fail_exec, strlen(fail_exec));
+               _exit(EXIT_FAILURE);
 
-       pid = fork();
-       if (pid == -1) {
-               close(sock[0]);
-               close(sock[1]);
-               weston_log("fork failed while launching '%s': %s\n",
-                          command, strerror(errno));
-               return NULL;
-       }
+       default:
+               close(wayland_socket.fds[1]);
+               ivi = weston_compositor_get_user_data(compositor);
+               client = wl_client_create(compositor->wl_display,
+                                         wayland_socket.fds[0]);
+               if (!client) {
+                       custom_env_fini(&child_env);
+                       close(wayland_socket.fds[0]);
+                       free(fail_exec);
+                       weston_log("client_launch: "
+                               "wl_client_create failed while launching '%s'.\n",
+                               path);
+                       return NULL;
+               }
 
-       if (pid == 0) {
-               client_exec(command, sock[1]);
-               _Exit(EXIT_FAILURE);
-       }
-       close(sock[1]);
+               proc->pid = pid;
+               proc->cleanup = cleanup;
+               wl_list_insert(&ivi->child_process_list, &proc->link);
+               break;
 
-       client = wl_client_create(ivi->compositor->wl_display, sock[0]);
-       if (!client) {
-               close(sock[0]);
-               weston_log("Failed to create wayland client for '%s'",
-                          command);
-               return NULL;
+       case -1:
+               fdstr_close_all(&wayland_socket);
+               weston_log("client_launch: "
+                          "fork failed while launching '%s': %s\n", path,
+                          strerror(errno));
+               break;
        }
 
+       custom_env_fini(&child_env);
+       free(fail_exec);
+
        return client;
 }
 
+struct process_info {
+       struct weston_process proc;
+       char *path;
+};
+
+static void
+process_handle_sigchld(struct weston_process *process, int status)
+{
+       struct process_info *pinfo =
+               container_of(process, struct process_info, proc);
+
+       /*
+        * There are no guarantees whether this runs before or after
+        * the wl_client destructor.
+        */
+
+       if (WIFEXITED(status)) {
+               weston_log("%s exited with status %d\n", pinfo->path,
+                          WEXITSTATUS(status));
+       } else if (WIFSIGNALED(status)) {
+               weston_log("%s died on signal %d\n", pinfo->path,
+                          WTERMSIG(status));
+       } else {
+               weston_log("%s disappeared\n", pinfo->path);
+       }
+
+       free(pinfo->path);
+       free(pinfo);
+}
+
 int
 ivi_launch_shell_client(struct ivi_compositor *ivi)
 {
+       struct process_info *pinfo;
        struct weston_config_section *section;
        char *command = NULL;
 
@@ -900,11 +969,28 @@ ivi_launch_shell_client(struct ivi_compositor *ivi)
        if (!command)
                return -1;
 
-       ivi->shell_client.client = launch_shell_client(ivi, command);
-       if (!ivi->shell_client.client)
+       pinfo = zalloc(sizeof *pinfo);
+       if (!pinfo)
                return -1;
 
+       pinfo->path = strdup(command);
+       if (!pinfo->path)
+               goto out_free;
+
+       ivi->shell_client.client = client_launch(ivi->compositor, &pinfo->proc,
+                                                command, process_handle_sigchld);
+       if (!ivi->shell_client.client)
+               goto out_str;
+
        return 0;
+
+out_str:
+       free(pinfo->path);
+
+out_free:
+       free(pinfo);
+
+       return -1;
 }
 
 static void