From 136a009ef606c2213be943a0fc6f79e9a95a61c7 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Mon, 14 Sep 2020 15:58:12 +0200 Subject: [PATCH 1/2] Terminate replayer processes when parent process unexpectedly terminates. --- cli/fossilize_replay_linux.hpp | 37 ++++++++++++++++++++++++--- fossilize_external_replayer_linux.hpp | 26 +++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/cli/fossilize_replay_linux.hpp b/cli/fossilize_replay_linux.hpp index 89d0141b..bad5c629 100644 --- a/cli/fossilize_replay_linux.hpp +++ b/cli/fossilize_replay_linux.hpp @@ -525,7 +525,7 @@ static int run_master_process(const VulkanDevice::Options &opts, // Create an epoll instance and add the signal fd to it. // The signalfd will signal when SIGCHLD is pending. - Global::epoll_fd = epoll_create(2 * int(processes) + 1); + Global::epoll_fd = epoll_create(2 * int(processes) + 2); if (Global::epoll_fd < 0) { LOGE("Failed to create epollfd. Too old Linux kernel?\n"); @@ -543,6 +543,25 @@ static int run_master_process(const VulkanDevice::Options &opts, } } + // If the master process is the process group master, then STDIN is used as a sentinel. + // If it closes, treat this as the parent process having terminated without + // cleanly shutting down the Fossilize process and we should take down this process group. + if (getpgrp() == getpid()) + { + LOGI("Using STDIN as sentinel FD.\n"); + + epoll_event event = {}; + event.events = 0; + event.data.u32 = UINT32_MAX - 1; + if (epoll_ctl(Global::epoll_fd, EPOLL_CTL_ADD, STDIN_FILENO, &event) < 0) + { + LOGE("Failed to add STDIN_FILENO to epoll.\n"); + return EXIT_FAILURE; + } + } + else + LOGI("Not using STDIN as sentinel FD.\n"); + // fork() and pipe() strategy. for (unsigned i = 0; i < processes; i++) { @@ -577,9 +596,9 @@ static int run_master_process(const VulkanDevice::Options &opts, for (int i = 0; i < ret; i++) { auto &e = events[i]; - if (e.events & (EPOLLIN | EPOLLRDHUP)) + if (e.events & (EPOLLIN | EPOLLHUP | EPOLLRDHUP)) { - if (e.data.u32 != UINT32_MAX) + if (e.data.u32 < UINT32_MAX - 1) { auto &proc = child_processes[e.data.u32 & 0x7fffffffu]; @@ -604,7 +623,7 @@ static int run_master_process(const VulkanDevice::Options &opts, } } } - else + else if (e.data.u32 == UINT32_MAX) { // Read from signalfd to clear the pending flag. signalfd_siginfo info = {}; @@ -643,6 +662,16 @@ static int run_master_process(const VulkanDevice::Options &opts, } } } + else if (e.data.u32 == UINT32_MAX - 1) + { + LOGI("Parent process died, terminating early ...\n"); + // STDIN in parent process was hung, kill every child process and ourselves and then exit. + if (killpg(getpid(), SIGKILL) < 0) + { + LOGE("Failed to kill process group ... This should not happen.\n"); + abort(); + } + } } else if (e.events & EPOLLERR) { diff --git a/fossilize_external_replayer_linux.hpp b/fossilize_external_replayer_linux.hpp index 93830a8c..43ad0336 100644 --- a/fossilize_external_replayer_linux.hpp +++ b/fossilize_external_replayer_linux.hpp @@ -59,6 +59,7 @@ struct ExternalReplayer::Impl pid_t pid = -1; int fd = -1; int kill_fd = -1; + int close_fd = -1; SharedControlBlock *shm_block = nullptr; size_t shm_block_size = 0; int wstatus = 0; @@ -95,6 +96,8 @@ ExternalReplayer::Impl::~Impl() close(fd); if (kill_fd >= 0) close(kill_fd); + if (close_fd >= 0) + close(close_fd); if (shm_block) munmap(shm_block, shm_block_size); @@ -111,6 +114,9 @@ void ExternalReplayer::Impl::reset_pid() if (kill_fd >= 0) close(kill_fd); kill_fd = -1; + if (close_fd >= 0) + close(close_fd); + close_fd = -1; } ExternalReplayer::PollResult ExternalReplayer::Impl::poll_progress(ExternalReplayer::Progress &progress) @@ -601,6 +607,10 @@ bool ExternalReplayer::Impl::start(const ExternalReplayer::Options &options) if (pipe(fds) < 0) return false; + int close_fds[2] = { -1, -1 }; + if (!options.inherit_process_group && pipe(close_fds) < 0) + return false; + pid_t new_pid = fork(); if (new_pid > 0) { @@ -609,18 +619,34 @@ bool ExternalReplayer::Impl::start(const ExternalReplayer::Options &options) fd = -1; pid = new_pid; kill_fd = fds[0]; + + if (!options.inherit_process_group) + { + close(close_fds[0]); + close_fd = close_fds[1]; + } } else if (new_pid == 0) { close(fds[0]); if (!options.inherit_process_group) { + close(close_fds[1]); + // Set the process group ID so we can kill all the child processes as needed. if (setpgid(0, 0) < 0) { LOGE("Failed to set PGID in child.\n"); exit(1); } + + if (dup2(close_fds[0], STDIN_FILENO) < 0) + { + LOGE("Failed to dup FD to stdin in child process.\n"); + exit(1); + } + + close(close_fds[0]); } // Notify parent process that it can return. From bf783696711f47e3b86f00ac202f91ab8f06853e Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Mon, 14 Sep 2020 16:05:57 +0200 Subject: [PATCH 2/2] Always assign processes to job handles on Windows. --- fossilize_external_replayer_windows.hpp | 29 +++++++++++-------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/fossilize_external_replayer_windows.hpp b/fossilize_external_replayer_windows.hpp index a43637c5..076adf81 100644 --- a/fossilize_external_replayer_windows.hpp +++ b/fossilize_external_replayer_windows.hpp @@ -575,24 +575,21 @@ bool ExternalReplayer::Impl::start(const ExternalReplayer::Options &options) si.hStdError = GetStdHandle(STD_ERROR_HANDLE); } - if (options.inherit_process_group) + job_handle = CreateJobObjectA(nullptr, nullptr); + if (!job_handle) { - job_handle = CreateJobObjectA(nullptr, nullptr); - if (!job_handle) - { - LOGE("Failed to create job handle.\n"); - // Not fatal, we just won't bother with this. - } - else + LOGE("Failed to create job handle.\n"); + // Not fatal, we just won't bother with this. + } + else + { + // Kill all child processes if the parent dies. + JOBOBJECT_EXTENDED_LIMIT_INFORMATION jeli = {}; + jeli.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + if (!SetInformationJobObject(job_handle, JobObjectExtendedLimitInformation, &jeli, sizeof(jeli))) { - // Kill all child processes if the parent dies. - JOBOBJECT_EXTENDED_LIMIT_INFORMATION jeli = {}; - jeli.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; - if (!SetInformationJobObject(job_handle, JobObjectExtendedLimitInformation, &jeli, sizeof(jeli))) - { - LOGE("Failed to set information for job object.\n"); - // Again, not fatal. - } + LOGE("Failed to set information for job object.\n"); + // Again, not fatal. } }