Library support for two-phase daemonization

[Andreas Gustafsson]

Hi all,

As discussed in PR 48282, some daemons background themselves before
opening the sockets or registering the RPCs they receive requests on.
This leads to race conditions: if you try to use the service provided
by a daemon such as nfsd immediately after starting it, you get random
failures because it may not yet be ready to provide service.

In some cases, this can be fixed by moving the call to daemon(3)
later, to a point where the socket/RPC setup is complete. But this
won't work in daemons that use kqueues or threads, because kqueues and
threads are not inherited across a fork.

I think the right way to fix the problem is to split the daemonization
into two phases: the daemon should fork early, before creating threads
or kqueues, but the parent process should only exit when the child is
ready to accept requests for service.

A proof of concept implementation of such a scheme is already in
src/tests/lib/libc/net/h_dns_server.c, and I would now like to move
this code into a library so that it can be used by other daemons, too.

I propose to do this by adding the two functions daemon2_fork() and
daemon2_detach() to libc. A patch containing my proposed changes is
at:

http://www.gson.org/netbsd/patches/daemon2-v9.patch

The patch includes updates to the daemon(3) man page and test cases.
Initially, two daemons would be switched to the new scheme: nfsd and
rquotad (the ones involved in the test failures reported in PR 48282);
others can follow later.

Comments? Objections?

Thanks,

[Joerg Sonnenberger]

Post by Andreas Gustafsson
Comments? Objections?

I don't like the approach. I would to just extend the existing daemon
interface slightly.

(1) daemon2() returns a filter descriptor. It is the responsibility of
the child to write '\0' to this fd and close it, once it is done with
initialisation.

(2) If the child writes a code other than '\0', it is interpreted as
error by the parent and used as exit status.

(3) A new argument provides the default exit code in case the child
terminates before writing the status byte.

Whether a timeout should be provided as fourth argument is a question I
can't answer right now.

Note that the existing daemon functionality can be obtained by just
closing the descriptor returned by daemon2.

Joerg

[Andreas Gustafsson]

Post by Joerg Sonnenberger
I don't like the approach.

Could you please elaborate on what aspects of it you dislike, and why?

Post by Joerg Sonnenberger
I would to just extend the existing daemon
interface slightly.
(1) daemon2() returns a filter descriptor.

I assume you mean a file descriptor.

Post by Joerg Sonnenberger
It is the responsibility of
the child to write '\0' to this fd and close it, once it is done with
initialisation.

Do you mean calling write() and close() directly rather than using a
library function specific to the task? IMO, having a library function
hiding the implementation details of the file descriptor is cleaner.

Would your daemon2() function also include the parts of daemon() that
(optinally) do chdir("/") and close stdin/stdout/stderr? I think you
would usually want to do those after setting up sockets and such, not
before, so that you can read configuration files using relative paths,
and report any errors that occur during the setup via stderr.

Post by Joerg Sonnenberger
(2) If the child writes a code other than '\0', it is interpreted as
error by the parent and used as exit status.
(3) A new argument provides the default exit code in case the child
terminates before writing the status byte.

Are there actually any daemons that distinguish between different
kinds of failures to start by means of the exit status? If there is a
genuine need for that functionality, I will be happy to add another
library function to support it, say daemon2_fail(int exit_status),
but I don't think it should be done by calling write() directly.

Post by Joerg Sonnenberger
Whether a timeout should be provided as fourth argument is a question I
can't answer right now.

I don't see a need for one.

Post by Joerg Sonnenberger
Note that the existing daemon functionality can be obtained by just
closing the descriptor returned by daemon2.

After passing 0 as your proposed default exit code argument, right?

In my proposal, the existing daemon() functionality can be obtained by
calling daemon2_detach() immediately after calling daemon2_fork().
I don't think either approach has an advantage over the other in that
regard.

[Joerg Sonnenberger]

Post by Andreas Gustafsson

Post by Joerg Sonnenberger
I don't like the approach.

Could you please elaborate on what aspects of it you dislike, and why?

It is the wrong abstraction and doesn't really save much compared to
just doing the dance by hand.

Post by Andreas Gustafsson

Post by Joerg Sonnenberger
I would to just extend the existing daemon
interface slightly.
(1) daemon2() returns a filter descriptor.

I assume you mean a file descriptor.

Yeah, typo.

Post by Andreas Gustafsson

Post by Joerg Sonnenberger
It is the responsibility of
the child to write '\0' to this fd and close it, once it is done with
initialisation.

Do you mean calling write() and close() directly rather than using a
library function specific to the task? IMO, having a library function
hiding the implementation details of the file descriptor is cleaner.

Providing a function is fine, but it is an important implementation
detail. I don't think hiding it is a good idea -- it would break with
closefrom(2) for example.

Post by Andreas Gustafsson
Would your daemon2() function also include the parts of daemon() that
(optinally) do chdir("/") and close stdin/stdout/stderr? I think you
would usually want to do those after setting up sockets and such, not
before, so that you can read configuration files using relative paths,
and report any errors that occur during the setup via stderr.

Most daemons that want to deal with that are already set up to do those
steps seperatedly and early. chdir(2) is only really useful in
combination with chroot(2) and the later requires enough care by itself.

Post by Andreas Gustafsson

Post by Joerg Sonnenberger
(2) If the child writes a code other than '\0', it is interpreted as
error by the parent and used as exit status.
(3) A new argument provides the default exit code in case the child
terminates before writing the status byte.

Are there actually any daemons that distinguish between different
kinds of failures to start by means of the exit status? If there is a
genuine need for that functionality, I will be happy to add another
library function to support it, say daemon2_fail(int exit_status),
but I don't think it should be done by calling write() directly.

If you want to communicate more than "something failed", the exit status
is certainly most helpful. As mentioned above, providing a function for
this purpose is fine.

Post by Andreas Gustafsson

Post by Joerg Sonnenberger
Note that the existing daemon functionality can be obtained by just
closing the descriptor returned by daemon2.

After passing 0 as your proposed default exit code argument, right?

Yes.

Joerg

[Andrew Shadura]

Hello,

On Tue, 28 Jan 2014 19:19:30 +0100

Post by Joerg Sonnenberger
(1) daemon2() returns a filter descriptor. It is the responsibility of
the child to write '\0' to this fd and close it, once it is done with
initialisation.
(2) If the child writes a code other than '\0', it is interpreted as
error by the parent and used as exit status.

First of all, I'm sorry for replying to a two months old thread, but
this seems to be similar to what Ian Jackson proposed in debian-ctte@
around the same time (Cc-ed to a bug report #733452)
:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=733452#1501

I think it's worth joining forces in developing a common daemon
readiness protocol.

[Andreas Gustafsson]

Post by Andrew Shadura
On Tue, 28 Jan 2014 19:19:30 +0100

Post by Joerg Sonnenberger
(1) daemon2() returns a filter descriptor. It is the responsibility of
the child to write '\0' to this fd and close it, once it is done with
initialisation.
(2) If the child writes a code other than '\0', it is interpreted as
error by the parent and used as exit status.

First of all, I'm sorry for replying to a two months old thread, but
around the same time (Cc-ed to a bug report #733452)
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=733452#1501

That discussion seems to be about non-forking daemons, while my
concern when starting the present thread was with forking daemons.

For forking daemons, there is already an established protocol for
determining readiness: simply wait for the parent process to exit.
My goal was only to simplify the implementation of that protocol,
not to define a new one. While the library functions I proposed
communicate through a file descriptor (with or without the changes
Joerg suggested), that communication does not form a readiness
protocol in the sense of the Debian discussion because it is all
internal to the daemon implementation.

Post by Andrew Shadura
I think it's worth joining forces in developing a common daemon
readiness protocol.

If you think NetBSD needs a readiness protocol for non-forking
daemons, that's fine, but I think it should be a separate discussion.

Regards,

[Christos Zoulas]

Post by Andreas Gustafsson
Hi all,
As discussed in PR 48282, some daemons background themselves before
opening the sockets or registering the RPCs they receive requests on.
This leads to race conditions: if you try to use the service provided
by a daemon such as nfsd immediately after starting it, you get random
failures because it may not yet be ready to provide service.
In some cases, this can be fixed by moving the call to daemon(3)
later, to a point where the socket/RPC setup is complete. But this
won't work in daemons that use kqueues or threads, because kqueues and
threads are not inherited across a fork.
I think the right way to fix the problem is to split the daemonization
into two phases: the daemon should fork early, before creating threads
or kqueues, but the parent process should only exit when the child is
ready to accept requests for service.
A proof of concept implementation of such a scheme is already in
src/tests/lib/libc/net/h_dns_server.c, and I would now like to move
this code into a library so that it can be used by other daemons, too.
I propose to do this by adding the two functions daemon2_fork() and
daemon2_detach() to libc. A patch containing my proposed changes is
http://www.gson.org/netbsd/patches/daemon2-v9.patch
The patch includes updates to the daemon(3) man page and test cases.
Initially, two daemons would be switched to the new scheme: nfsd and
rquotad (the ones involved in the test failures reported in PR 48282);
others can follow later.
Comments? Objections?

- the assumption that pfd[0] + 1 == pfd[1] is not true. If you start
with 0, 1, 2 open then close 1, and issue pipe(), you'll end up
with pfd[0] = 1, pfd[1] = 3. So you should test that both pfd[0]
and pfd[1] are > STDERR_FILENO.
- I think most fd's should be close on exec here. so you could use pipe2()
to setup sigpipe and close-on-exec handlign.
- results of read/write are ssize_t
- I like to write fork as a switch instead of ifthenelif, r is pid_t.
- why isn't the detach daemon pipe static?

christos

[Andreas Gustafsson]

Post by Christos Zoulas
- the assumption that pfd[0] + 1 == pfd[1] is not true. If you start
with 0, 1, 2 open then close 1, and issue pipe(), you'll end up
with pfd[0] = 1, pfd[1] = 3. So you should test that both pfd[0]
and pfd[1] are > STDERR_FILENO.

The code is not making that assumption. It is only trying to protect
the write end of the pipe (pfd[1]) from getting a fd in the range 0-2,
not the read end, as the read end will be closed anyway by the time
the child closes fds 0-2.

Post by Christos Zoulas
- I think most fd's should be close on exec here. so you could use pipe2()
to setup sigpipe and close-on-exec handlign.
- results of read/write are ssize_t
- I like to write fork as a switch instead of ifthenelif, r is pid_t.
- why isn't the detach daemon pipe static?

I'll fix these.

Implementation nits aside, do you find the API acceptable, or would
you prefer to expose the file descriptor as Joerg is suggesting?

[Christos Zoulas]

Post by Andreas Gustafsson

Post by Christos Zoulas
- the assumption that pfd[0] + 1 == pfd[1] is not true. If you start
with 0, 1, 2 open then close 1, and issue pipe(), you'll end up
with pfd[0] = 1, pfd[1] = 3. So you should test that both pfd[0]
and pfd[1] are > STDERR_FILENO.

The code is not making that assumption. It is only trying to protect
the write end of the pipe (pfd[1]) from getting a fd in the range 0-2,
not the read end, as the read end will be closed anyway by the time
the child closes fds 0-2.

Ok, sounds good.

Post by Andreas Gustafsson

Post by Christos Zoulas
- I think most fd's should be close on exec here. so you could use pipe2()
to setup sigpipe and close-on-exec handlign.
- results of read/write are ssize_t
- I like to write fork as a switch instead of ifthenelif, r is pid_t.
- why isn't the detach daemon pipe static?

I'll fix these.
Implementation nits aside, do you find the API acceptable, or would
you prefer to expose the file descriptor as Joerg is suggesting?

I think that exposing the file descriptor is nice because it avoids the
static storage issue.

christos

[Anthony Mallet]

On Wednesday, at 19:29, Christos Zoulas wrote:
| In article <***@guava.gson.org>,
| >Implementation nits aside, do you find the API acceptable, or would
| >you prefer to expose the file descriptor as Joerg is suggesting?
|
| I think that exposing the file descriptor is nice because it avoids the
| static storage issue.

But this makes the API more cumbersome.

What about a sem_open(3) for the synchronization, initially empty and
sem_post'ed by the child?
The semantics may be clearer, but sadly this requires -lrt.

Or maybe just a kill(SIGHUP) to the parent?

[Anthony Mallet]

On Thursday, at 09:23, Anthony Mallet wrote:
|
| Or maybe just a kill(SIGHUP) to the parent?

More details: I've seen this pattern in X.org (IIRC) and I'm using this without
any issue so far:

/* daemonize */
if (daemonize) {
switch(fork()) {
case -1:
perror("cannot fork");
exit(2);

case 0: /* child */ {
int fd;

if (setsid() == -1) exit(-1);
if (chdir("/")) /*noop*/;

fd = open("/dev/null", O_RDWR, 0);
if (fd != -1) {
(void)dup2(fd, STDIN_FILENO);
if (fd > STDERR_FILENO)
(void)close(fd);
}
signal(SIGUSR1, SIG_IGN);
}

default: /* parent */ {
sigset_t sset;
int sig = 0;

sigemptyset(&sset);
sigaddset(&sset, SIGUSR1);
sigprocmask(SIG_BLOCK, &sset, NULL);
do { sigwait(&sset, &sig); } while (sig == 0);
assert(sig == SIGUSR1);
_exit(0);
}
}
}

handler = signal(SIGUSR1, SIG_IGN);
/* if SIGUSR1 is set to SIG_IGN, it was either set above or inherited from
* the parent (because we did not fiddle with signal handlers in the child
* yet). So either:
* - the parent really ignores it
* - the parent is in the 'parent' branch above
*/

/* do initialization stuff etc ... then signal the parent */

if (handler == SIG_IGN)
kill(getppid(), SIGUSR1);

[Andreas Gustafsson]

Post by Anthony Mallet
More details: I've seen this pattern in X.org (IIRC) and I'm using this without
/* daemonize */
if (daemonize) {
switch(fork()) {
perror("cannot fork");
exit(2);
case 0: /* child */ {
int fd;
if (setsid() == -1) exit(-1);
if (chdir("/")) /*noop*/;
fd = open("/dev/null", O_RDWR, 0);
if (fd != -1) {
(void)dup2(fd, STDIN_FILENO);
if (fd > STDERR_FILENO)
(void)close(fd);
}
signal(SIGUSR1, SIG_IGN);
}

Missing a "break;" here?

Post by Anthony Mallet
default: /* parent */ {
sigset_t sset;
int sig = 0;
sigemptyset(&sset);
sigaddset(&sset, SIGUSR1);
sigprocmask(SIG_BLOCK, &sset, NULL);
do { sigwait(&sset, &sig); } while (sig == 0);
assert(sig == SIGUSR1);
_exit(0);
}
}
}
handler = signal(SIGUSR1, SIG_IGN);
/* if SIGUSR1 is set to SIG_IGN, it was either set above or inherited from
* the parent (because we did not fiddle with signal handlers in the child
* - the parent really ignores it
* - the parent is in the 'parent' branch above
*/
/* do initialization stuff etc ... then signal the parent */
if (handler == SIG_IGN)
kill(getppid(), SIGUSR1);

I think this suffers from a similar race condition as Iain's code;
the SIGUSR1 may be delivered to the parent before it has called
sigprocmask().

[Anthony Mallet]

On Friday, at 21:02, Andreas Gustafsson wrote:
| Missing a "break;" here?

Yes, sorry. I cut too much of unrelated code...

| > if (handler == SIG_IGN)
| > kill(getppid(), SIGUSR1);
|
| I think this suffers from a similar race condition as Iain's code;
| the SIGUSR1 may be delivered to the parent before it has called
| sigprocmask().

I agree, and I must admit I had overlooked this, so thanks for the feedback.

I choosed this approach so that the program can be used according two distinct
use cases:
- classic standalone mode, start within a shell script: daemonize with a
command line switch and return to the shell only once the program is ready.

- start from another unrelated "launcher" program. The launcher must of course
implement the 'SIGUSR1' logic. In this mode, the child does not fork itself,
but it is still able to inform it's parent about it 'ready' state. (think
for instance about inetd, although it's not inetd in my use case).

So the pipe() approach does not really fit in my case. Or at least I don't see
an easy way to communicate a file descriptor the child.

However, I think the race that you mention can be fixed by moving the
sigprocmask() and related code before the fork(), and restoring the former mask
in the child. What do you think of the following?

[Andreas Gustafsson]

Post by Anthony Mallet
I choosed this approach so that the program can be used according two distinct
- classic standalone mode, start within a shell script: daemonize with a
command line switch and return to the shell only once the program is ready.
- start from another unrelated "launcher" program. The launcher must of course
implement the 'SIGUSR1' logic. In this mode, the child does not fork itself,
but it is still able to inform it's parent about it 'ready' state. (think
for instance about inetd, although it's not inetd in my use case).
So the pipe() approach does not really fit in my case. Or at least I don't see
an easy way to communicate a file descriptor the child.

Is this just meant as background to explain why your code example is
structured the way it is, or are you suggesting that NetBSD should
have library support for this pattern in addition to the traditional
daemonization pattern?

Post by Anthony Mallet
However, I think the race that you mention can be fixed by moving the
sigprocmask() and related code before the fork(), and restoring the former mask
in the child. What do you think of the following?

Looks correct to me, but signal semantics are not really my speciality.

[Anthony Mallet]

On Sunday, at 12:40, Andreas Gustafsson wrote:
| Anthony Mallet wrote:
| > I choosed this approach so that the program can be used according two distinct
| > use cases:
| > - classic standalone mode, start within a shell script: daemonize with a
| > command line switch and return to the shell only once the program is ready.
| >
| > - start from another unrelated "launcher" program. The launcher must of course
| > implement the 'SIGUSR1' logic. In this mode, the child does not fork itself,
| > but it is still able to inform it's parent about it 'ready' state. (think
| > for instance about inetd, although it's not inetd in my use case).
| >
| > So the pipe() approach does not really fit in my case. Or at least I don't see
| > an easy way to communicate a file descriptor the child.
|
| Is this just meant as background to explain why your code example is
| structured the way it is, or are you suggesting that NetBSD should
| have library support for this pattern in addition to the traditional
| daemonization pattern?

Yes, I was suggesting this pattern as an alternative implementation of your
daemon_{fork,detach} with pipe().

Your suggestion solves one problem (the first use case above), and since you
are planning to add this to libc, it may be worth trying to solve the other use
case at the same time?

The API could be something like:
- daemon_fork()
- daemon_detach()

- daemon_spawn(): basically a posix_spawn() with the additional signal
notification pattern.

daemon_spawn() could of course only be used with programs using the
daemon_{fork,detach} API, which is maybe an argument against my proposal.

[Andreas Gustafsson]

Post by Anthony Mallet
Yes, I was suggesting this pattern as an alternative implementation of your
daemon_{fork,detach} with pipe().
Your suggestion solves one problem (the first use case above), and since you
are planning to add this to libc, it may be worth trying to solve the other use
case at the same time?
- daemon_fork()
- daemon_detach()
- daemon_spawn(): basically a posix_spawn() with the additional signal
notification pattern.

I'm afraid I still don't see why this is needed. Is there currently
code in NetBSD using this pattern? What advantages does it offer over
traditional daemonization?

Also, I'm specifically not trying to redesign or replace the existing
daemonization API, but just to provide the minimum functionality
needed to fix race conditions in existing NetBSD daemons. If you
believe that can be better achieved with signals than with pipes, I'm
interested in hearing your arguments, but I'm not really interested in
solving any unrelated problems.

[Christos Zoulas]

Post by Anthony Mallet

Post by Anthony Mallet
Yes, I was suggesting this pattern as an alternative implementation of your
daemon_{fork,detach} with pipe().
Your suggestion solves one problem (the first use case above), and since you
are planning to add this to libc, it may be worth trying to solve the

other use

Post by Anthony Mallet
case at the same time?
- daemon_fork()
- daemon_detach()
- daemon_spawn(): basically a posix_spawn() with the additional signal
notification pattern.

I'm afraid I still don't see why this is needed. Is there currently
code in NetBSD using this pattern? What advantages does it offer over
traditional daemonization?

This pattern is necessary for any threaded deamon and has been open-coded
in all such daemons (see dhclient, named, etc.)

Post by Anthony Mallet
Also, I'm specifically not trying to redesign or replace the existing
daemonization API, but just to provide the minimum functionality
needed to fix race conditions in existing NetBSD daemons. If you
believe that can be better achieved with signals than with pipes, I'm
interested in hearing your arguments, but I'm not really interested in
solving any unrelated problems.

file descriptors are better than signals because they can pass state (where
that is needed) and offer better control semantics (like a parent that
can spawn many children that waits until all of them have been initialized
before it exits). This is one more reason to pass the file descriptor
back to the parent like joerg suggested.

christos

[Andreas Gustafsson]

Post by Christos Zoulas

Post by Andreas Gustafsson
I'm afraid I still don't see why this is needed. Is there currently
code in NetBSD using this pattern? What advantages does it offer over
traditional daemonization?

This pattern is necessary for any threaded deamon and has been open-coded
in all such daemons (see dhclient, named, etc.)

Are we talking about the same pattern? I was referring to the latter
of Anthony's two use cases:

| > I choosed this approach so that the program can be used according two distinct
| > use cases:
| > - classic standalone mode, start within a shell script: daemonize with a
| > command line switch and return to the shell only once the program is ready.
| >
| > - start from another unrelated "launcher" program. The launcher must of course
| > implement the 'SIGUSR1' logic. In this mode, the child does not fork itself,
| > but it is still able to inform it's parent about it 'ready' state. (think
| > for instance about inetd, although it's not inetd in my use case).

That latter pattern is not used in dhclient nor named.

Post by Christos Zoulas
file descriptors are better than signals because they can pass state (where
that is needed) and offer better control semantics

Agreed.

Post by Christos Zoulas
(like a parent that
can spawn many children that waits until all of them have been initialized
before it exits).

Yes, pipes are better for that, too, but I don't think that case is common
enough to warrant library support.

Post by Christos Zoulas
This is one more reason to pass the file descriptor back to the
parent like joerg suggested.

I don't see any such proposal from Joerg - he proposed returning the
file descriptor from deaemon2(), which amounts to passing it to the
child, not the parent.

[Christos Zoulas]

On Feb 3, 10:07am, ***@gson.org (Andreas Gustafsson) wrote:
-- Subject: Re: Library support for two-phase daemonization

| Christos Zoulas wrote:
| > In article <***@guava.gson.org>,
| > Andreas Gustafsson <***@gson.org> wrote:
| > >I'm afraid I still don't see why this is needed. Is there currently
| > >code in NetBSD using this pattern? What advantages does it offer over
| > >traditional daemonization?
| >
| > This pattern is necessary for any threaded deamon and has been open-coded
| > in all such daemons (see dhclient, named, etc.)
|
| Are we talking about the same pattern? I was referring to the latter
| of Anthony's two use cases:

I was talking about the file descriptor write.

| | > I choosed this approach so that the program can be used according two distinct
| | > use cases:
| | > - classic standalone mode, start within a shell script: daemonize with a
| | > command line switch and return to the shell only once the program is ready.
| | >
| | > - start from another unrelated "launcher" program. The launcher must of course
| | > implement the 'SIGUSR1' logic. In this mode, the child does not fork itself,
| | > but it is still able to inform it's parent about it 'ready' state. (think
| | > for instance about inetd, although it's not inetd in my use case).
|
| That latter pattern is not used in dhclient nor named.
|
| > file descriptors are better than signals because they can pass state (where
| > that is needed) and offer better control semantics
|
| Agreed.

Good :-)

| > (like a parent that
| > can spawn many children that waits until all of them have been initialized
| > before it exits).
|
| Yes, pipes are better for that, too, but I don't think that case is common
| enough to warrant library support.

It is a generalization of the single child case.

| > This is one more reason to pass the file descriptor back to the
| > parent like joerg suggested.
|
| I don't see any such proposal from Joerg - he proposed returning the
| file descriptor from deaemon2(), which amounts to passing it to the
| child, not the parent.

I think the easiest pattern is to fork early, do all the work in the child,
then have the parent exit once it is notified that the child is done.

christos

[Andreas Gustafsson]

Post by Christos Zoulas
| > (like a parent that
| > can spawn many children that waits until all of them have been initialized
| > before it exits).
|
| Yes, pipes are better for that, too, but I don't think that case is common
| enough to warrant library support.
It is a generalization of the single child case.

That may be, but unless you can show some actual use cases in NetBSD,
I'd rather not complicate the API to support that generalization.

[Joerg Sonnenberger]

Post by Andreas Gustafsson

Post by Christos Zoulas
This is one more reason to pass the file descriptor back to the
parent like joerg suggested.

I don't see any such proposal from Joerg - he proposed returning the
file descriptor from deaemon2(), which amounts to passing it to the
child, not the parent.

Correct. In my proposal, the daemonize function would not return in the
parent and call (_)exit when done.

Joerg

[Anthony Mallet]

On Sunday, at 17:51, Andreas Gustafsson wrote:
| > The API could be something like:
| > - daemon_fork()
| > - daemon_detach()
| >
| > - daemon_spawn(): basically a posix_spawn() with the additional signal
| > notification pattern.
|
| I'm afraid I still don't see why this is needed. Is there currently
| code in NetBSD using this pattern? What advantages does it offer over
| traditional daemonization?

I think this is needed as soon as you want to spawn a process, via
posix_spawn() or fork()/exec(), and you want to be notified of the proper
startup/initialization of the process. This is slightly different from
daemonizing. When daemonizing, the parent and child run the same code (so you
control both "sides" and it's easier to communicate).

The libc/kernel offer no standard way of doing this, while there is for
instance the standard SIGCHLD and wait(2) for joining a dead child. So I felt
there was a potential for filling this gap and propose a clean pattern for
that. But maybe not :)

| Also, I'm specifically not trying to redesign or replace the existing
| daemonization API, but just to provide the minimum functionality
| needed to fix race conditions in existing NetBSD daemons. If you
| believe that can be better achieved with signals than with pipes, I'm
| interested in hearing your arguments, but I'm not really interested in
| solving any unrelated problems.

Yes. The problem I am raising is definitely not NetBSD specific at all :)

[Mouse]

Post by Anthony Mallet
I think this is needed as soon as you want to spawn a process, via
posix_spawn() or fork()/exec(), and you want to be notified of the
proper startup/initialization of the process.

Since "proper startup" is a program-specific notion (eg, some programs
need to load config files), this cannot be done without help from the
program being run.

If you're satisfied with exec() succeeding, the pattern I usually use
is to create an AF_LOCAL/SOCK_STREAM socketpair, set the child's end
close-on-exec, and exec. If the exec fails, I then (in the child)
write errno to the socketpair and exit. The parent then does a recv()
with MSG_WAITALL. If it gets zero, the exec worked; if it gets
sizeof(int), it can report that errno. If it gets anything else, my
code usually just coughs and dies; I've never actually seen it happen.
(I'm leaving out lots of details, such as closing the parent's end of
the socketpair by the child.)

I don't like signal-based approaches, especially for library routines,
both because they conflict with other uses for the signal(s) used and
because they make it somewhere between much harder and impossible to
pass the errno from a failed exec back to the parent. (If the child is
still connected to a useful error-reporting channel, the child can
report it directly. This is not always the case.)

/~\ The ASCII Mouse
\ / Ribbon Campaign
X Against HTML ***@rodents-montreal.org
/ \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B

[Iain Hibbert]

Post by Andreas Gustafsson
I think the right way to fix the problem is to split the daemonization
into two phases: the daemon should fork early, before creating threads
or kqueues, but the parent process should only exit when the child is
ready to accept requests for service.

btw I did this in btpand(8), which uses libevent (and thus kqueue)

Post by Andreas Gustafsson
The patch includes updates to the daemon(3) man page and test cases.
Initially, two daemons would be switched to the new scheme: nfsd and
rquotad (the ones involved in the test failures reported in PR 48282);
others can follow later.
Comments?

one minor nit from me is that syscalls such as pipe(2), fork(2) and
read(2) return "-1" for error according to the manpages .. using a test
for "< 0" to detect that never seems right to me, especially when casting
from a different type which could involve a truncation (eg ssize_t)

Also, I agree with christos that a switch is better :)

iain

[Andreas Gustafsson]

Post by Iain Hibbert
btw I did this in btpand(8), which uses libevent (and thus kqueue)

That's good, but I think it has a race condition: the SIGUSR1 can be
delivered to the parent before it has called signal(SIGUSR1, main_exit).

Post by Iain Hibbert
one minor nit from me is that syscalls such as pipe(2), fork(2) and
read(2) return "-1" for error according to the manpages .. using a test
for "< 0" to detect that never seems right to me,

Testing for < 0 is a long-standing tradition which I will honor
as long as /usr/share/misc/style does. :)

Post by Iain Hibbert
especially when casting from a different type which could involve a
truncation (eg ssize_t)

Checking a truncated value is wrong whether you check for < 0 or == -1.
I will fix the truncation.

[Iain Hibbert]

Post by Andreas Gustafsson

Post by Iain Hibbert
btw I did this in btpand(8), which uses libevent (and thus kqueue)

That's good, but I think it has a race condition: the SIGUSR1 can be
delivered to the parent before it has called signal(SIGUSR1, main_exit).

hm.. I meant to add, that having a standard API for such as this would
make it easier to do without making mistakes :)

For some reason, I have an uncommitted change in my local source tree, to
use SIGTERM for this.. which would at least always cause an exit. (I don't
remember why I made that change, have not looked at it for a while as my
Android phone stinks at Bluetooth)

iain

[Andreas Gustafsson]

Post by Iain Hibbert
For some reason, I have an uncommitted change in my local source tree, to
use SIGTERM for this.. which would at least always cause an exit.

The exit should already happen, as the default action of SIGUSR1 is to
terminate the process, just like SIGTERM. But the process spawning
the daemon may be confused when wait() indicates that it exited due to
a signal rather than successfully.

[Alan Barrett]

Post by Andreas Gustafsson
I propose to do this by adding the two functions daemon2_fork() and
daemon2_detach() to libc. A patch containing my proposed changes is
http://www.gson.org/netbsd/patches/daemon2-v9.patch

Please could you show how to implement daemon() in terms of daemon2_*.

Could the args be passed to daemon2_fork() instead of to
daemon2_detach(), to make it easier to convert code from using daemon()
to daemon2_*()?

If we are redesigning this interface, then perhaps we should go
further, and provide a richer list of things to do in between
the fork and the detach. Some possible things one might want to
do are: setuid, chroot, chdir(somewhere other than "/"),
anything that posix_spawn() can do.

--apb (Alan Barrett)

[Andreas Gustafsson]

Post by Alan Barrett
Please could you show how to implement daemon() in terms of daemon2_*.

int daemon(int nochdir, int noclose) {
int r = daemon_fork();
if (r == -1)
return -1;
return daemon2_detach(nochdir, noclose);
}

Post by Alan Barrett
Could the args be passed to daemon2_fork() instead of to
daemon2_detach(), to make it easier to convert code from using daemon()
to daemon2_*()?

I think it is clearer to pass them to the function where they are
actually used.

Post by Alan Barrett
If we are redesigning this interface, then perhaps we should go
further, and provide a richer list of things to do in between
the fork and the detach. Some possible things one might want to
do are: setuid, chroot, chdir(somewhere other than "/"),
anything that posix_spawn() can do.

I'm not trying to do a redesign so much as a minimal extension of
the existing design to solve a specific problem.

If we were to actually do a redesign, it would be nice if we could
avoid having function arguments whose name starts with "no".