From nobody Sun Apr 09 23:58:02 2023 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Pvpvb45gQz44Srh; Sun, 9 Apr 2023 23:58:03 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Pvpvb19Nxz4FWt; Sun, 9 Apr 2023 23:58:03 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1681084683; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=edslcl/WMp5i2IDkvyHd7EG1ZZhombQcCUtpvW/J2bU=; b=w+0aBaBD3fwzX3RrbcOKm3ja9c9E0wXZm/uesvlHwOE1vetRe0fhhffyOMkkY7GSQhhw6u gxP38eSWj4/eKQNrDD8L0rKLX4FTesG4fp8uQMIv6EzGi5bdhzOzBYQxoxBjoGOblD+XQ8 BHvBBQeAxPiP/nCYNe1pcB874zY9ACajUkR2OIaZGJh/0Ne7+e6RcTS+GxjEmew7UvoAG/ P/PknSXvU1KIlqh8Rsl/EPU5RB6ayNnBhxykoAj/S57LfX80YIOh5uIgcIVkRrHf32TXws lKnFzCGKjN+bgQeY+4wvxRATrNR5C9woQQRv3r2gXPPTrwxMnm12kgDZmdNFmg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1681084683; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=edslcl/WMp5i2IDkvyHd7EG1ZZhombQcCUtpvW/J2bU=; b=Lqg0KGkUIPEcjgaiEAqCJWyLM9n7HawmPmyPs8tX4Uga2b9Y/SPUPNHZZDwJthgrUGBLTB xgDa3kxtZq1Nk4/QEcvDA6ixNe/ERS0+5U0A59Fwn/hIW9ggtqK31YSY6xlqK/Y0GaSbTs IjtLhihV2QdsxFvIWvgsNQSRy0alEeinKCiMgzVQe5BVQhHHUUS09tk7OtynD/LwSSjejh 1Wp6zhKSes3DDfXWv/Kcq3VP6guSSnxCBbAgbzsppCY2K6EbmNW/GGeqI2uiKR1gZ1zyuh O5yYQWDKwufm1otu535pX0cW5rwl/j7gjbTQjrxX+bx5Fro84OLmN9zB32DCuA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1681084683; a=rsa-sha256; cv=none; b=D4K0yDjbJYyxUSEdKJsTz0ijb9TXcrMGd2viwo3VyYx8OuwYlPN2kHOCDaoe7svBeBxHLq K/8dHzbYR1uugtiVxR/awn0n2oS00jD/czRjLna7S7jSuF3SPIxJtNnVMTwEa6TV3vlvq+ Llx/M8MMBoxbrKpEskz2u2VTrFVQlJJ6T0RmR2Gj3OxEGgCx9n2ZB0er/EWPZbueKG62Wb Z2Z1IUssMECtjoPAGZl3HutAG3HrnZUGNCcpZCoXCktDKDQviSl3s2nepqe2gH92l6S6cS 2lVwfE+kiG5b9X8ScdxeERrePJ7mJAYClQt9Avrpx9qdk6SoSQKEg1Gj/v7iAQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4PvpvZ4KmfzSch; Sun, 9 Apr 2023 23:58:02 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 339Nw2ST012039; Sun, 9 Apr 2023 23:58:02 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 339Nw2R6012038; Sun, 9 Apr 2023 23:58:02 GMT (envelope-from git) Date: Sun, 9 Apr 2023 23:58:02 GMT Message-Id: <202304092358.339Nw2R6012038@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kyle Evans Subject: git: 81f89a76e8fa - stable/13 - daemon: decouple init logic from main loop List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kevans X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 81f89a76e8fa9a9fba8f8cfdf44addf8bae2d448 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=81f89a76e8fa9a9fba8f8cfdf44addf8bae2d448 commit 81f89a76e8fa9a9fba8f8cfdf44addf8bae2d448 Author: Ihor Antonov AuthorDate: 2023-03-23 02:37:12 +0000 Commit: Kyle Evans CommitDate: 2023-04-09 22:49:52 +0000 daemon: decouple init logic from main loop main() func contained both initialization and main loop logic. This made certain operations like restarting problematic and required dirty hacks in form of goto jumps. This commit moves the main loop logic into daemon_eventloop(), cleans up main, and makes restart logic clear: daemon_mainloop() is run in a loop with a restart condition checked at the end. Reviewed by: kevans (cherry picked from commit 4c41f4a0d67fc93cfb07ad5287f02d024d19ef5a) --- usr.sbin/daemon/daemon.c | 195 ++++++++++++++++++++++++++--------------------- 1 file changed, 109 insertions(+), 86 deletions(-) diff --git a/usr.sbin/daemon/daemon.c b/usr.sbin/daemon/daemon.c index 983dbdc7ed8c..4b218c2b53c5 100644 --- a/usr.sbin/daemon/daemon.c +++ b/usr.sbin/daemon/daemon.c @@ -60,7 +60,12 @@ __FBSDID("$FreeBSD$"); #define LBUF_SIZE 4096 struct daemon_state { + sigset_t mask_orig; + sigset_t mask_read; + sigset_t mask_term; + sigset_t mask_susp; int pipe_fd[2]; + char **argv; const char *child_pidfile; const char *parent_pidfile; const char *output_filename; @@ -96,6 +101,7 @@ static void open_pid_files(struct daemon_state *); static void do_output(const unsigned char *, size_t, struct daemon_state *); static void daemon_sleep(time_t, long); static void daemon_state_init(struct daemon_state *); +static void daemon_eventloop(struct daemon_state *); static void daemon_terminate(struct daemon_state *); static volatile sig_atomic_t terminate = 0; @@ -163,47 +169,9 @@ main(int argc, char *argv[]) char *p = NULL; int ch = 0; struct daemon_state state; - sigset_t mask_orig; - sigset_t mask_read; - sigset_t mask_term; - sigset_t mask_susp; daemon_state_init(&state); - /* - * Signal handling logic: - * - * - SIGTERM is masked while there is no child. - * - * - SIGCHLD is masked while reading from the pipe. SIGTERM has to be - * caught, to avoid indefinite blocking on read(). - * - * - Both SIGCHLD and SIGTERM are masked before calling sigsuspend() - * to avoid racing. - * - * - After SIGTERM is recieved and propagated to the child there are - * several options on what to do next: - * - read until EOF - * - read until EOF but only for a while - * - bail immediately - * Currently the third option is used, because otherwise there is no - * guarantee that read() won't block indefinitely if the child refuses - * to depart. To handle the second option, a different approach - * would be needed (procctl()?). - * - * - Child's exit might be detected by receiveing EOF from the pipe. - * But the child might have closed its stdout and stderr, so deamon - * must wait for the SIGCHLD to ensure that the child is actually gone. - */ - sigemptyset(&mask_susp); - sigemptyset(&mask_read); - sigemptyset(&mask_term); - sigemptyset(&mask_orig); - sigaddset(&mask_susp, SIGTERM); - sigaddset(&mask_susp, SIGCHLD); - sigaddset(&mask_term, SIGTERM); - sigaddset(&mask_read, SIGCHLD); - /* * Supervision mode is enabled if one of the following options are used: * --child-pidfile -p @@ -309,6 +277,7 @@ main(int argc, char *argv[]) } argc -= optind; argv += optind; + state.argv = argv; if (argc == 0) { usage(1); @@ -343,8 +312,8 @@ main(int argc, char *argv[]) pidfile_write(state.parent_pidfh); if (state.supervision_enabled) { - /* Block SIGTERM to avoid racing until we have forked. */ - if (sigprocmask(SIG_BLOCK, &mask_term, &mask_orig)) { + /* Block SIGTERM to avoid racing until the child is spawned. */ + if (sigprocmask(SIG_BLOCK, &state.mask_term, &state.mask_orig)) { warn("sigprocmask"); daemon_terminate(&state); } @@ -357,8 +326,50 @@ main(int argc, char *argv[]) * not have superuser privileges. */ (void)madvise(NULL, 0, MADV_PROTECT); -restart: - if (pipe(state.pipe_fd)) { + } + do { + daemon_eventloop(&state); + close(state.pipe_fd[0]); + state.pipe_fd[0] = -1; + } while (state.restart_enabled && !terminate); + + daemon_terminate(&state); +} + + +/* + * Main event loop: fork the child and watch for events. + * In legacy mode simply execve into the target process. + * + * Signal handling logic: + * + * - SIGTERM is masked while there is no child. + * + * - SIGCHLD is masked while reading from the pipe. SIGTERM has to be + * caught, to avoid indefinite blocking on read(). + * + * - Both SIGCHLD and SIGTERM are masked before calling sigsuspend() + * to avoid racing. + * + * - After SIGTERM is recieved and propagated to the child there are + * several options on what to do next: + * - read until EOF + * - read until EOF but only for a while + * - bail immediately + * Currently the third option is used, because otherwise there is no + * guarantee that read() won't block indefinitely if the child refuses + * to depart. To handle the second option, a different approach + * would be needed (procctl()?). + * + * - Child's exit might be detected by receiveing EOF from the pipe. + * But the child might have closed its stdout and stderr, so deamon + * must wait for the SIGCHLD to ensure that the child is actually gone. + */ +static void +daemon_eventloop(struct daemon_state *state) +{ + if (state->supervision_enabled) { + if (pipe(state->pipe_fd)) { err(1, "pipe"); } /* @@ -371,43 +382,43 @@ restart: /* fork failed, this can only happen when supervision is enabled */ if (pid == -1) { warn("fork"); - daemon_terminate(&state); + daemon_terminate(state); } /* fork succeeded, this is child's branch or supervision is disabled */ if (pid == 0) { - pidfile_write(state.child_pidfh); + pidfile_write(state->child_pidfh); - if (state.user != NULL) { - restrict_process(state.user); + if (state->user != NULL) { + restrict_process(state->user); } /* * In supervision mode, the child gets the original sigmask, * and dup'd pipes. */ - if (state.supervision_enabled) { - close(state.pipe_fd[0]); - if (sigprocmask(SIG_SETMASK, &mask_orig, NULL)) { + if (state->supervision_enabled) { + close(state->pipe_fd[0]); + if (sigprocmask(SIG_SETMASK, &state->mask_orig, NULL)) { err(1, "sigprogmask"); } - if (state.stdmask & STDERR_FILENO) { - if (dup2(state.pipe_fd[1], STDERR_FILENO) == -1) { + if (state->stdmask & STDERR_FILENO) { + if (dup2(state->pipe_fd[1], STDERR_FILENO) == -1) { err(1, "dup2"); } } - if (state.stdmask & STDOUT_FILENO) { - if (dup2(state.pipe_fd[1], STDOUT_FILENO) == -1) { + if (state->stdmask & STDOUT_FILENO) { + if (dup2(state->pipe_fd[1], STDOUT_FILENO) == -1) { err(1, "dup2"); } } - if (state.pipe_fd[1] != STDERR_FILENO && - state.pipe_fd[1] != STDOUT_FILENO) { - close(state.pipe_fd[1]); + if (state->pipe_fd[1] != STDERR_FILENO && + state->pipe_fd[1] != STDOUT_FILENO) { + close(state->pipe_fd[1]); } } - execvp(argv[0], argv); + execvp(state->argv[0], state->argv); /* execvp() failed - report error and exit this process */ - err(1, "%s", argv[0]); + err(1, "%s", state->argv[0]); } /* @@ -417,64 +428,65 @@ restart: * * Unblock SIGTERM - now there is a valid child PID to signal to. */ - if (sigprocmask(SIG_UNBLOCK, &mask_term, NULL)) { + if (sigprocmask(SIG_UNBLOCK, &state->mask_term, NULL)) { warn("sigprocmask"); - daemon_terminate(&state); + daemon_terminate(state); } - close(state.pipe_fd[1]); - state.pipe_fd[1] = -1; + close(state->pipe_fd[1]); + state->pipe_fd[1] = -1; - setproctitle("%s[%d]", state.title, (int)pid); + setproctitle("%s[%d]", state->title, (int)pid); for (;;) { - if (child_gone && state.child_eof) { + if (child_gone && state->child_eof) { break; } if (terminate) { - daemon_terminate(&state); + daemon_terminate(state); } - if (state.child_eof) { - if (sigprocmask(SIG_BLOCK, &mask_susp, NULL)) { + if (state->child_eof) { + if (sigprocmask(SIG_BLOCK, &state->mask_susp, NULL)) { warn("sigprocmask"); - daemon_terminate(&state); + daemon_terminate(state); } while (!terminate && !child_gone) { - sigsuspend(&mask_orig); + sigsuspend(&state->mask_orig); } - if (sigprocmask(SIG_UNBLOCK, &mask_susp, NULL)) { + if (sigprocmask(SIG_UNBLOCK, &state->mask_susp, NULL)) { warn("sigprocmask"); - daemon_terminate(&state); + daemon_terminate(state); } continue; } - if (sigprocmask(SIG_BLOCK, &mask_read, NULL)) { + if (sigprocmask(SIG_BLOCK, &state->mask_read, NULL)) { warn("sigprocmask"); - daemon_terminate(&state); + daemon_terminate(state); } - state.child_eof = !listen_child(state.pipe_fd[0], &state); + state->child_eof = !listen_child(state->pipe_fd[0], state); - if (sigprocmask(SIG_UNBLOCK, &mask_read, NULL)) { + if (sigprocmask(SIG_UNBLOCK, &state->mask_read, NULL)) { warn("sigprocmask"); - daemon_terminate(&state); + daemon_terminate(state); } } - if (state.restart_enabled && !terminate) { - daemon_sleep(state.restart_delay, 0); - } - if (sigprocmask(SIG_BLOCK, &mask_term, NULL)) { + + /* + * At the end of the loop the the child is already gone. + * Block SIGTERM to avoid racing until the child is spawned. + */ + if (sigprocmask(SIG_BLOCK, &state->mask_term, NULL)) { warn("sigprocmask"); - daemon_terminate(&state); + daemon_terminate(state); } - if (state.restart_enabled && !terminate) { - close(state.pipe_fd[0]); - state.pipe_fd[0] = -1; - goto restart; + + /* sleep before exiting mainloop if restart is enabled */ + if (state->restart_enabled && !terminate) { + daemon_sleep(state->restart_delay, 0); } - daemon_terminate(&state); } static void @@ -746,6 +758,7 @@ daemon_state_init(struct daemon_state *state) { *state = (struct daemon_state) { .pipe_fd = { -1, -1 }, + .argv = NULL, .parent_pidfh = NULL, .child_pidfh = NULL, .child_pidfile = NULL, @@ -767,6 +780,16 @@ daemon_state_init(struct daemon_state *state) .output_fd = -1, .output_filename = NULL, }; + + sigemptyset(&state->mask_susp); + sigemptyset(&state->mask_read); + sigemptyset(&state->mask_term); + sigemptyset(&state->mask_orig); + sigaddset(&state->mask_susp, SIGTERM); + sigaddset(&state->mask_susp, SIGCHLD); + sigaddset(&state->mask_term, SIGTERM); + sigaddset(&state->mask_read, SIGCHLD); + } static _Noreturn void