From df9008bfc4aba3715044f88fe3ae9ee441b0143b Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 7 Sep 2022 11:46:46 +0200 Subject: [PATCH] StartUnixWorker(): watch forked child via waitpid(), not SIGCHLD handler Before: On SIGCHLD from the forked worker the umbrella process sets a failure flag. StartUnixWorker() recognises that and does waitpid(), failure message, etc.. On OpenBSD we can't tell the signal source, so we always set the failure flag. That's not how our IPC shall work, that breaks the IPC sooner or later. After: No SIGCHLD handling and no failure flag setting. Instead StartUnixWorker()'s wait loop uses waitpid(x,y,WNOHANG) to avoid false positives while watching the forked worker. --- lib/cli/daemoncommand.cpp | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/lib/cli/daemoncommand.cpp b/lib/cli/daemoncommand.cpp index fb1dd0289..f3c7d7895 100644 --- a/lib/cli/daemoncommand.cpp +++ b/lib/cli/daemoncommand.cpp @@ -381,13 +381,6 @@ static void UmbrellaSignalHandler(int num, siginfo_t *info, void*) l_CurrentlyStartingUnixWorkerState.store(UnixWorkerState::LoadedConfig); } break; - case SIGCHLD: - if (l_CurrentlyStartingUnixWorkerState.load() == UnixWorkerState::Pending - && (info->si_pid == 0 || info->si_pid == l_CurrentlyStartingUnixWorkerPid.load()) ) { - // The seamless worker currently being started by StartUnixWorker() failed - l_CurrentlyStartingUnixWorkerState.store(UnixWorkerState::Failed); - } - break; case SIGINT: case SIGTERM: // Someone requested our termination @@ -570,28 +563,21 @@ static pid_t StartUnixWorker(const std::vector& configs, bool close NotifyWatchdog(); #endif /* HAVE_SYSTEMD */ - switch (l_CurrentlyStartingUnixWorkerState.load()) { - case UnixWorkerState::LoadedConfig: - Log(LogNotice, "cli") - << "Worker process successfully loaded its config"; - break; - case UnixWorkerState::Failed: - Log(LogNotice, "cli") - << "Worker process couldn't load its config"; + if (waitpid(pid, nullptr, WNOHANG) > 0) { + Log(LogNotice, "cli") + << "Worker process couldn't load its config"; - while (waitpid(pid, nullptr, 0) == -1 && errno == EINTR) { -#ifdef HAVE_SYSTEMD - NotifyWatchdog(); -#endif /* HAVE_SYSTEMD */ - } - pid = -2; - break; - default: - Utility::Sleep(0.2); - continue; + pid = -2; + break; } - break; + if (l_CurrentlyStartingUnixWorkerState.load() == UnixWorkerState::LoadedConfig) { + Log(LogNotice, "cli") + << "Worker process successfully loaded its config"; + break; + } + + Utility::Sleep(0.2); } // Reset flags for the next time