From 7860bce26191c80239cc93db4d903f2cd0faa51b Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Tue, 14 Dec 2021 16:43:41 -0800 Subject: [PATCH] server: listen to SIGCHLD to reap workers No more confusing zombie processes. Signed-off-by: Chia-I Wu Reviewed-by: Yiwei Zhang Reviewed-by: Ryan Neph --- server/render_server.c | 64 ++++++++++++++++++++++++++++++++--- server/render_worker.c | 77 +++++++++++++++++++++++++++++++++++++++++- server/render_worker.h | 5 ++- 3 files changed, 140 insertions(+), 6 deletions(-) diff --git a/server/render_server.c b/server/render_server.c index bc55388..6b129e4 100644 --- a/server/render_server.c +++ b/server/render_server.c @@ -5,7 +5,9 @@ #include "render_server.h" +#include #include +#include #include #include "render_client.h" @@ -13,16 +15,70 @@ #define RENDER_SERVER_MAX_WORKER_COUNT 256 +enum render_server_poll_type { + RENDER_SERVER_POLL_SOCKET = 0, + RENDER_SERVER_POLL_SIGCHLD /* optional */, + RENDER_SERVER_POLL_COUNT, +}; + +static int +render_server_init_poll_fds(struct render_server *srv, + struct pollfd poll_fds[static RENDER_SERVER_POLL_COUNT]) +{ + const int socket_fd = srv->client->socket.fd; + const int sigchld_fd = render_worker_jail_get_sigchld_fd(srv->worker_jail); + + poll_fds[RENDER_SERVER_POLL_SOCKET] = (const struct pollfd){ + .fd = socket_fd, + .events = POLLIN, + }; + poll_fds[RENDER_SERVER_POLL_SIGCHLD] = (const struct pollfd){ + .fd = sigchld_fd, + .events = POLLIN, + }; + + return sigchld_fd >= 0 ? 2 : 1; +} + +static bool +render_server_poll(UNUSED struct render_server *srv, + struct pollfd *poll_fds, + int poll_fd_count) +{ + int ret; + do { + ret = poll(poll_fds, poll_fd_count, -1); + } while (ret < 0 && (errno == EINTR || errno == EAGAIN)); + + if (ret <= 0) { + render_log("failed to poll in the main loop"); + return false; + } + + return true; +} + static bool render_server_run(struct render_server *srv) { + struct render_client *client = srv->client; + + struct pollfd poll_fds[RENDER_SERVER_POLL_COUNT]; + const int poll_fd_count = render_server_init_poll_fds(srv, poll_fds); + while (srv->state == RENDER_SERVER_STATE_RUN) { - struct render_client *client = srv->client; - if (!render_client_dispatch(client)) + if (!render_server_poll(srv, poll_fds, poll_fd_count)) return false; - /* TODO this should be triggered by SIGCHLD */ - render_worker_jail_reap_workers(srv->worker_jail); + if (poll_fds[RENDER_SERVER_POLL_SOCKET].revents) { + if (!render_client_dispatch(client)) + return false; + } + + if (poll_fds[RENDER_SERVER_POLL_SIGCHLD].revents) { + if (!render_worker_jail_reap_workers(srv->worker_jail)) + return false; + } } return true; diff --git a/server/render_worker.c b/server/render_worker.c index b7343dd..35ef1d0 100644 --- a/server/render_worker.c +++ b/server/render_worker.c @@ -21,8 +21,10 @@ #error "no worker defined" #endif +#include #include #include +#include #include #include #include @@ -33,6 +35,7 @@ struct minijail; struct render_worker_jail { int max_worker_count; + int sigchld_fd; struct minijail *minijail; struct list_head workers; @@ -156,6 +159,36 @@ fork_minijail(const struct minijail *template) #endif /* ENABLE_RENDER_SERVER_WORKER_MINIJAIL */ +#ifndef ENABLE_RENDER_SERVER_WORKER_THREAD + +static int +create_sigchld_fd(void) +{ + const int signum = SIGCHLD; + + sigset_t set; + if (sigemptyset(&set) || sigaddset(&set, signum)) { + render_log("failed to initialize sigset_t"); + return -1; + } + + int fd = signalfd(-1, &set, SFD_NONBLOCK | SFD_CLOEXEC); + if (fd < 0) { + render_log("failed to create signalfd"); + return -1; + } + + if (sigprocmask(SIG_BLOCK, &set, NULL)) { + render_log("failed to call sigprocmask"); + close(fd); + return -1; + } + + return fd; +} + +#endif /* !ENABLE_RENDER_SERVER_WORKER_THREAD */ + static void render_worker_jail_add_worker(struct render_worker_jail *jail, struct render_worker *worker) @@ -211,8 +244,15 @@ render_worker_jail_create(int max_worker_count, return NULL; jail->max_worker_count = max_worker_count; + jail->sigchld_fd = -1; list_inithead(&jail->workers); +#ifndef ENABLE_RENDER_SERVER_WORKER_THREAD + jail->sigchld_fd = create_sigchld_fd(); + if (jail->sigchld_fd < 0) + goto fail; +#endif + #if defined(ENABLE_RENDER_SERVER_WORKER_MINIJAIL) jail->minijail = create_minijail(seccomp_filter, seccomp_path); if (!jail->minijail) @@ -253,12 +293,45 @@ render_worker_jail_destroy(struct render_worker_jail *jail) minijail_destroy(jail->minijail); #endif + if (jail->sigchld_fd >= 0) + close(jail->sigchld_fd); + free(jail); } -void +int +render_worker_jail_get_sigchld_fd(const struct render_worker_jail *jail) +{ + return jail->sigchld_fd; +} + +static bool +render_worker_jail_drain_sigchld_fd(struct render_worker_jail *jail) +{ + if (jail->sigchld_fd < 0) + return true; + + do { + struct signalfd_siginfo siginfos[8]; + const ssize_t r = read(jail->sigchld_fd, siginfos, sizeof(siginfos)); + if (r == sizeof(siginfos)) + continue; + if (r > 0 || (r < 0 && errno == EAGAIN)) + break; + + render_log("failed to read signalfd"); + return false; + } while (true); + + return true; +} + +bool render_worker_jail_reap_workers(struct render_worker_jail *jail) { + if (!render_worker_jail_drain_sigchld_fd(jail)) + return false; + do { struct render_worker *worker = render_worker_jail_reap_any_worker(jail, false /* block */); @@ -269,6 +342,8 @@ render_worker_jail_reap_workers(struct render_worker_jail *jail) if (worker->destroyed) render_worker_jail_remove_worker(jail, worker); } while (true); + + return true; } void diff --git a/server/render_worker.h b/server/render_worker.h index 15ae9c0..c18938b 100644 --- a/server/render_worker.h +++ b/server/render_worker.h @@ -26,7 +26,10 @@ render_worker_jail_create(int max_worker_count, void render_worker_jail_destroy(struct render_worker_jail *jail); -void +int +render_worker_jail_get_sigchld_fd(const struct render_worker_jail *jail); + +bool render_worker_jail_reap_workers(struct render_worker_jail *jail); void