Skip to content

supervise-daemon: make logger a child of the supervisor - #1103

Open
navi-desu wants to merge 2 commits into
masterfrom
logger_chld
Open

navi-desu wants to merge 2 commits into
masterfrom
logger_chld

Conversation

@navi-desu

Copy link
Copy Markdown
Member

No description provided.

Comment thread src/supervise-daemon/supervise-daemon.c Resolved
having the logger be a child of the supervisor not only means we can't
respawn it, but causes stopgroup to crash some services, as killing the
logger causes sigpipe

@N-R-K N-R-K left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I don't fully understand what problem is being solved here. Do you have a concrete example?

if ((fd[1] = xopen_nonblock(redirect, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) == -1)
eerrorx("%s: unable to redirect the logfile for stdout '%s': %s", applet, redirect, strerror(errno));
} else if (process) {
pipe2(fd, O_CLOEXEC);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should error check.

@navi-desu

navi-desu commented Oct 31, 2026 •

Copy link
Copy Markdown
Member Author

TBH I don't fully understand what problem is being solved here. Do you have a concrete example?

two things:

a) so far loggers were not supervised, could crash and give random EPIPEs to the socket, with this, the supervisor holds the pipe so EPIPE wouldn't happen, and is actually supervising and respawning the logger

b) we hadded stopgroup=yes, which sends a signal to -pid instead of pid; the whole process group, before this the logger would be on that process group and if it's killed before the main process, they'd also get EPIPE trying to log anything at shutdown

having the supervisor actually keep track of loggers means we can kill them after the service is stopped and avoid that

plus, it's just cleaner to not give unexpected child processes to daemons

@N-R-K

N-R-K commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

I see. That makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants