Date: Thu, 28 Sep 2023 15:52:34 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: fe233250d6fd - main - syslogd: Do not reap child processes Message-ID: <202309281552.38SFqYHt022025@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=fe233250d6fd0d4b915c89244f2b1f79cb45284e commit fe233250d6fd0d4b915c89244f2b1f79cb45284e Author: Jake Freeland <jfree@FreeBSD.org> AuthorDate: 2023-09-01 02:50:19 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2023-09-28 15:51:53 +0000 syslogd: Do not reap child processes Use the SA_NOCLDWAIT sigaction() flag to disable the creation of zombie processes. This leaves no reason to wait() on children, so remove the reaping routine. Previously, deadq entries were removed in reapchild(). Move deadq removal into markit() after SIGKILL is sent. Reviewed by: markj MFC after: 3 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D41415 --- usr.sbin/syslogd/syslogd.c | 101 +++++---------------------------------------- 1 file changed, 11 insertions(+), 90 deletions(-) diff --git a/usr.sbin/syslogd/syslogd.c b/usr.sbin/syslogd/syslogd.c index fff45372fda8..c20928d48495 100644 --- a/usr.sbin/syslogd/syslogd.c +++ b/usr.sbin/syslogd/syslogd.c @@ -441,7 +441,6 @@ static void cfline(const char *, const char *, const char *, const char *); static const char *cvthname(struct sockaddr *); static void deadq_enter(int); static void deadq_remove(struct deadq_entry *); -static bool deadq_removebypid(pid_t); static int decode(const char *, const CODE *); static void die(int) __dead2; static void dofsync(void); @@ -453,7 +452,6 @@ static void init(bool); static void logerror(const char *); static void logmsg(int, const struct logtime *, const char *, const char *, const char *, const char *, const char *, const char *, int); -static void log_deadchild(pid_t, int, const char *); static void markit(void); static struct socklist *socksetup(struct addrinfo *, const char *, mode_t); static int socklist_recv_file(struct socklist *); @@ -466,7 +464,6 @@ static int prop_filter_compile(struct prop_filter *pfilter, static void parsemsg(const char *, char *); static void printsys(char *); static int p_open(const char *, pid_t *); -static void reapchild(int); static const char *ttymsg_check(struct iovec *, int, char *, int); static void usage(void); static bool validate(struct sockaddr *, const char *); @@ -598,6 +595,7 @@ addfile(int fd) int main(int argc, char *argv[]) { + struct sigaction act = { }; sigset_t sigset = { }; struct kevent ev; struct socklist *sl; @@ -846,6 +844,14 @@ main(int argc, char *argv[]) } (void)alarm(TIMERINTVL); + /* Do not create zombie processes. */ + act.sa_flags = SA_NOCLDWAIT; + if (sigaction(SIGCHLD, &act, NULL) == -1) { + warn("failed to apply signal handler"); + pidfile_remove(pfh); + exit(1); + } + /* tuck my process id away */ pidfile_write(pfh); @@ -884,9 +890,6 @@ main(int argc, char *argv[]) case SIGALRM: markit(); break; - case SIGCHLD: - reapchild(ev.ident); - break; } break; } @@ -2256,38 +2259,6 @@ ttymsg_check(struct iovec *iov, int iovcnt, char *line, int tmout) return (ttymsg(iov, iovcnt, line, tmout)); } -static void -reapchild(int signo __unused) -{ - int status; - pid_t pid, pipe_pid; - struct filed *f; - - while ((pid = wait3(&status, WNOHANG, (struct rusage *)NULL)) > 0) { - /* First, look if it's a process from the dead queue. */ - if (deadq_removebypid(pid)) - continue; - - /* Now, look in list of active processes. */ - STAILQ_FOREACH(f, &fhead, next) { - if (f->f_type == F_PIPE) { - if (pdgetpid(f->fu_pipe_pd, &pipe_pid) == -1) { - dprintf("Failed to query PID: %s\n", - strerror(errno)); - continue; - } - if (pipe_pid != pid) - continue; - /* Do not enter into deadq. */ - f->fu_pipe_pd = -1; - close_filed(f); - log_deadchild(pid, status, f->fu_pipe_pname); - break; - } - } - } -} - /* * Return a printable representation of a host address. */ @@ -3228,19 +3199,11 @@ markit(void) switch (dq->dq_timeout) { case 0: /* Already signalled once, try harder now. */ - if (pdkill(dq->dq_procdesc, SIGKILL) != 0) - (void)deadq_remove(dq); + (void)pdkill(dq->dq_procdesc, SIGKILL); + (void)deadq_remove(dq); break; case 1: - /* - * Timed out on dead queue, send terminate - * signal. Note that we leave the removal - * from the dead queue to reapchild(), which - * will also log the event (unless the process - * didn't even really exist, in case we simply - * drop it from the dead queue). - */ if (pdkill(dq->dq_procdesc, SIGTERM) != 0) (void)deadq_remove(dq); else @@ -3715,48 +3678,6 @@ deadq_remove(struct deadq_entry *dq) free(dq); } -static bool -deadq_removebypid(pid_t pid) -{ - struct deadq_entry *dq; - pid_t dq_pid; - - TAILQ_FOREACH(dq, &deadq_head, dq_entries) { - if (pdgetpid(dq->dq_procdesc, &dq_pid) == -1) { - dprintf("Failed to query PID: %s\n", strerror(errno)); - continue; - } - if (dq_pid == pid) { - deadq_remove(dq); - return (true); - } - } - return (false); -} - -static void -log_deadchild(pid_t pid, int status, const char *name) -{ - int code; - char buf[256]; - const char *reason; - - errno = 0; /* Keep strerror() stuff out of logerror messages. */ - if (WIFSIGNALED(status)) { - reason = "due to signal"; - code = WTERMSIG(status); - } else { - reason = "with status"; - code = WEXITSTATUS(status); - if (code == 0) - return; - } - (void)snprintf(buf, sizeof buf, - "Logging subprocess %d (%s) exited %s %d.", - pid, name, reason, code); - logerror(buf); -} - static struct socklist * socksetup(struct addrinfo *ai, const char *name, mode_t mode) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202309281552.38SFqYHt022025>