Attachment 259275 Details for Bug 277959 – git(1) diff against releng/14.2

[patch] git(1) diff against releng/14.2

0001-daemon-stop-rebuilding-the-kqueue-every-restart-of-t.patch (text/plain), 6.33 KB, created by Kyle Evans on 2025-04-02 13:15:53 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle Evans

Created: 2025-04-02 13:15:53 UTC

Size: 6.33 KB

patch

obsolete

>From e5e3d5457295fdfdc95eead15f87d0107070de84 Mon Sep 17 00:00:00 2001
>From: Kyle Evans <kevans@FreeBSD.org>
>Date: Tue, 19 Nov 2024 13:51:27 -0600
>Subject: [PATCH] daemon: stop rebuilding the kqueue every restart of the child
>
>We populate the kqueue with all of four kevents: three signal handlers and
>one for read of the child pipe.  Every time we start the child, we rebuild
>this kqueue from scratch for the child and tear it down before we exit and
>check if we need to restart the child.  As a consequence, we effectively
>drop any of the signals we're interested in between restarts.
>
>Push the kqueue out into the daemon state to avoid losing any signal events
>in the process, and reimplement the restart timer in terms of kqueue timers.
>The pipe read event will be automatically deleted upon last close, which
>leaves us with only the signal events that really get retained between
>restarts of the child.
>
>PR:		277959
>Reviewed by:	des, markj
>
>(cherry picked from commit bc1dfc316a2bba97773a14b96f5e976a52524be4)
>(cherry picked from commit 7ea2874eadf901b1187772670169b6fc3a44d917)
>---
> usr.sbin/daemon/daemon.c | 121 ++++++++++++++++++++++++++++++++-------
> 1 file changed, 101 insertions(+), 20 deletions(-)
>
>diff --git a/usr.sbin/daemon/daemon.c b/usr.sbin/daemon/daemon.c
>index 52fbfca1dcd2..411929e6e4df 100644
>--- a/usr.sbin/daemon/daemon.c
>+++ b/usr.sbin/daemon/daemon.c
>@@ -79,6 +79,7 @@ struct daemon_state {
> 	enum daemon_mode mode;
> 	int pid;
> 	int keep_cur_workdir;
>+	int kqueue_fd;
> 	int restart_delay;
> 	int stdmask;
> 	int syslog_priority;
>@@ -104,6 +105,7 @@ static void daemon_terminate(struct daemon_state *);
> static void daemon_exec(struct daemon_state *);
> static bool daemon_is_child_dead(struct daemon_state *);
> static void daemon_set_child_pipe(struct daemon_state *);
>+static int daemon_setup_kqueue(void);
> 
> static const char shortopts[] = "+cfHSp:P:ru:o:s:l:t:m:R:T:h";
> 
>@@ -322,6 +324,8 @@ main(int argc, char *argv[])
> 	/* Write out parent pidfile if needed. */
> 	pidfile_write(state.parent_pidfh);
> 
>+	state.kqueue_fd = daemon_setup_kqueue();
>+
> 	do {
> 		state.mode = MODE_SUPERVISE;
> 		daemon_eventloop(&state);
>@@ -377,27 +381,13 @@ daemon_eventloop(struct daemon_state *state)
> 		err(1, "pipe");
> 	}
> 
>-	kq = kqueuex(KQUEUE_CLOEXEC);
>+	kq = state->kqueue_fd;
> 	EV_SET(&event, state->pipe_fd[0], EVFILT_READ, EV_ADD|EV_CLEAR, 0, 0,
> 	    NULL);
> 	if (kevent(kq, &event, 1, NULL, 0, NULL) == -1) {
> 		err(EXIT_FAILURE, "failed to register kevent");
> 	}
> 
>-	EV_SET(&event, SIGHUP,  EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
>-	if (kevent(kq, &event, 1, NULL, 0, NULL) == -1) {
>-		err(EXIT_FAILURE, "failed to register kevent");
>-	}
>-
>-	EV_SET(&event, SIGTERM, EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
>-	if (kevent(kq, &event, 1, NULL, 0, NULL) == -1) {
>-		err(EXIT_FAILURE, "failed to register kevent");
>-	}
>-
>-	EV_SET(&event, SIGCHLD, EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
>-	if (kevent(kq, &event, 1, NULL, 0, NULL) == -1) {
>-		err(EXIT_FAILURE, "failed to register kevent");
>-	}
> 	memset(&event, 0, sizeof(struct kevent));
> 
> 	/* Spawn a child to exec the command. */
>@@ -490,28 +480,86 @@ daemon_eventloop(struct daemon_state *state)
> 			}
> 			continue;
> 		default:
>+			assert(0 && "Unexpected kevent filter type");
> 			continue;
> 		}
> 	}
> 
>-	close(kq);
>+	/* EVFILT_READ kqueue filter goes away here. */
> 	close(state->pipe_fd[0]);
> 	state->pipe_fd[0] = -1;
> }
> 
>+/*
>+ * Note that daemon_sleep() should not be called with anything but the signal
>+ * events in the kqueue without further consideration.
>+ */
> static void
> daemon_sleep(struct daemon_state *state)
> {
>-	struct timespec ts = { state->restart_delay, 0 };
>+	struct kevent event = { 0 };
>+	int ret;
>+
>+	assert(state->pipe_fd[0] == -1);
>+	assert(state->pipe_fd[1] == -1);
> 
> 	if (!state->restart_enabled) {
> 		return;
> 	}
>-	while (nanosleep(&ts, &ts) == -1) {
>-		if (errno != EINTR) {
>-			err(1, "nanosleep");
>+
>+	EV_SET(&event, 0, EVFILT_TIMER, EV_ADD|EV_ONESHOT, NOTE_SECONDS,
>+	    state->restart_delay, NULL);
>+	if (kevent(state->kqueue_fd, &event, 1, NULL, 0, NULL) == -1) {
>+		err(1, "failed to register timer");
>+	}
>+
>+	for (;;) {
>+		ret = kevent(state->kqueue_fd, NULL, 0, &event, 1, NULL);
>+		if (ret == -1) {
>+			if (errno != EINTR) {
>+				err(1, "kevent");
>+			}
>+
>+			continue;
>+		}
>+
>+		/*
>+		 * Any other events being raised are indicative of a problem
>+		 * that we need to investigate.  Most likely being that
>+		 * something was not cleaned up from the eventloop.
>+		 */
>+		assert(event.filter == EVFILT_TIMER ||
>+		    event.filter == EVFILT_SIGNAL);
>+
>+		if (event.filter == EVFILT_TIMER) {
>+			/* Break's over, back to work. */
>+			break;
>+		}
>+
>+		/* Process any pending signals. */
>+		switch (event.ident) {
>+		case SIGTERM:
>+			/*
>+			 * We could disarm the timer, but we'll be terminating
>+			 * promptly anyways.
>+			 */
>+			state->restart_enabled = false;
>+			return;
>+		case SIGHUP:
>+			if (state->log_reopen && state->output_fd >= 0) {
>+				reopen_log(state);
>+			}
>+
>+			break;
>+		case SIGCHLD:
>+		default:
>+			/* Discard */
>+			break;
> 		}
> 	}
>+
>+	/* SIGTERM should've returned immediately. */
>+	assert(state->restart_enabled);
> }
> 
> static void
>@@ -701,6 +749,7 @@ daemon_state_init(struct daemon_state *state)
> 		.restart_enabled = false,
> 		.pid = 0,
> 		.keep_cur_workdir = 1,
>+		.kqueue_fd = -1,
> 		.restart_delay = 1,
> 		.stdmask = STDOUT_FILENO | STDERR_FILENO,
> 		.syslog_enabled = false,
>@@ -719,6 +768,9 @@ daemon_terminate(struct daemon_state *state)
> {
> 	assert(state != NULL);
> 
>+	if (state->kqueue_fd >= 0) {
>+		close(state->kqueue_fd);
>+	}
> 	if (state->output_fd >= 0) {
> 		close(state->output_fd);
> 	}
>@@ -788,3 +840,32 @@ daemon_set_child_pipe(struct daemon_state *state)
> 	/* The child gets dup'd pipes. */
> 	close(state->pipe_fd[0]);
> }
>+
>+static int
>+daemon_setup_kqueue(void)
>+{
>+	int kq;
>+	struct kevent event = { 0 };
>+
>+	kq = kqueuex(KQUEUE_CLOEXEC);
>+	if (kq == -1) {
>+		err(EXIT_FAILURE, "kqueue");
>+	}
>+
>+	EV_SET(&event, SIGHUP,  EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
>+	if (kevent(kq, &event, 1, NULL, 0, NULL) == -1) {
>+		err(EXIT_FAILURE, "failed to register kevent");
>+	}
>+
>+	EV_SET(&event, SIGTERM, EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
>+	if (kevent(kq, &event, 1, NULL, 0, NULL) == -1) {
>+		err(EXIT_FAILURE, "failed to register kevent");
>+	}
>+
>+	EV_SET(&event, SIGCHLD, EVFILT_SIGNAL, EV_ADD, 0, 0, NULL);
>+	if (kevent(kq, &event, 1, NULL, 0, NULL) == -1) {
>+		err(EXIT_FAILURE, "failed to register kevent");
>+	}
>+
>+	return (kq);
>+}
>-- 
>2.48.1
>

Actions: View | Diff

Attachments on bug 277959: 259275 | 259276