waitpid() thread race

[Brian Candler]

I have a question about the semantics of wait()/waitpid().

My understanding is, as soon as wait() returns, the process is gone from the
process table, and therefore another fork() on the system could immediately
re-use the same PID. Is that correct?

Now let's suppose I have a program which forks children when it needs them.
It maintains a datastructure which is a hash of { pid => info }

Let's say there's a separate thread which blocks on a wait() call, and once
it has gotten the pid it updates this data structure to remove the entry for
<pid>

Now, it seems to me there is a race condition here: between wait() returning
and the <pid> entry being removed from the data structure, the main program
may have forked off another child with the same <pid>

Protecting the 'wait' and 'fork' threads with a mutex doesn't help. If I
lock the mutex before calling wait() then I prevent all forks for an
indefinite period of time; if I lock the mutex after calling wait() then the
race still exists, as the forking thread may already have the mutex and be
in the process of forking another child with the same pid.

So, what's the best way to handle this? Options I can think of are:

(1) Polling.

- lock mutex
- call waitpid(-1, 0, WNOHANG)
- update the data structure
- unlock mutex
- sleep 100ms
- go back to start

This seems rather icky.

(2) Modify the data structure to allow for the unlikely, but possible,
situation of having two processes with the same PID: one which has just been
reaped, and one which has just been forked. The reap process then removes
the first entry for the PID returned from wait().

This gives a messy datastructure just for handling this edge case.

(3) If there were an option to waitpid() which could tell you the pid of a
terminated process *without* reaping it, then it becomes easy:

- waitpid(-1, 0, WNOWAIT)
- update the data structure to remove the entry for this pid
- waitpid(pid, 0, 0) to remove it from the process table

It looks like Linux has a waitid() call with a WNOWAIT option, but I can't
see anything in the wait manpage for OpenBSD (4.0) which works this way.

Any other suggestions as to the best way to avoid this problem? I'm sure
this must be old ground :-)

Thanks,

Brian.

[Darrin Chandler]

Post by Brian Candler
I have a question about the semantics of wait()/waitpid().
My understanding is, as soon as wait() returns, the process is gone from the
process table, and therefore another fork() on the system could immediately
re-use the same PID. Is that correct?
Now let's suppose I have a program which forks children when it needs them.
It maintains a datastructure which is a hash of { pid => info }
Let's say there's a separate thread which blocks on a wait() call, and once
it has gotten the pid it updates this data structure to remove the entry for
<pid>
Now, it seems to me there is a race condition here: between wait() returning
and the <pid> entry being removed from the data structure, the main program
may have forked off another child with the same <pid>

If you've got that problem you already have other problems. Breeding and
reaping children with associated shared data structures is asking for
trouble unless you synchronize. If you solve that so that you're either
spawning a child OR reaping a child (never BOTH), then the pid reuse
isn't a problem anyway. And you do need to solve that, or you're going
to end up with mangled data.

Post by Brian Candler
Protecting the 'wait' and 'fork' threads with a mutex doesn't help. If I
lock the mutex before calling wait() then I prevent all forks for an
indefinite period of time; if I lock the mutex after calling wait() then the
race still exists, as the forking thread may already have the mutex and be
in the process of forking another child with the same pid.
(1) Polling.
- lock mutex
- call waitpid(-1, 0, WNOHANG)
- update the data structure
- unlock mutex
- sleep 100ms
- go back to start
This seems rather icky.
(2) Modify the data structure to allow for the unlikely, but possible,
situation of having two processes with the same PID: one which has just been
reaped, and one which has just been forked. The reap process then removes
the first entry for the PID returned from wait().
This gives a messy datastructure just for handling this edge case.
(3) If there were an option to waitpid() which could tell you the pid of a
- waitpid(-1, 0, WNOWAIT)
- update the data structure to remove the entry for this pid
- waitpid(pid, 0, 0) to remove it from the process table
It looks like Linux has a waitid() call with a WNOWAIT option, but I can't
see anything in the wait manpage for OpenBSD (4.0) which works this way.
Any other suggestions as to the best way to avoid this problem? I'm sure
this must be old ground :-)
Thanks,
Brian.

[Philip Guenther]

On 4/7/07, Brian Candler <***@pobox.com> wrote:
...

Post by Brian Candler
Let's say there's a separate thread which blocks on a wait() call, and once
it has gotten the pid it updates this data structure to remove the entry for
<pid>
Now, it seems to me there is a race condition here: between wait() returning
and the <pid> entry being removed from the data structure, the main program
may have forked off another child with the same <pid>
Protecting the 'wait' and 'fork' threads with a mutex doesn't help. If I
lock the mutex before calling wait() then I prevent all forks for an
indefinite period of time; if I lock the mutex after calling wait() then the
race still exists, as the forking thread may already have the mutex and be
in the process of forking another child with the same pid.
(1) Polling.

...

Post by Brian Candler
(2) Modify the data structure <...>

...

Post by Brian Candler
(3) If there were an option to waitpid() which could tell you the pid of a

...

Post by Brian Candler
Any other suggestions as to the best way to avoid this problem? I'm sure
this must be old ground :-)

Instead of separating the obtaining of the pid from the actual
reaping, you can instead separate the blocking from the return of the
pid+reaping. That lets you lock the datastructure only when you know
wait() won't block. To block until a child is ready to be reaped, use
SIGCHLD, blocking it when you aren't ready, ala:

volatile sig_atomic_t saw_sigchld;
sigset_t orig_sigset

void handle_sigchld(int sig)
{
saw_sigchld = 1;
}

/* do this once before creating any threads, so that they're all
blocking SIGCHLD */
int init(void)
{
struct sigaction sa;
sigset_t set;

sigemptyset(&sa.sa_mask);
sa.sa_flags = 0;
sa.sa_handler = &handle_sigchld;
if (sigaction(SIGCHLD, &sa, NULL))
return errno;

sigemptyset(&set);
sigaddset(&set, SIGCHLD);
return pthread_sigmask(SIG_BLOCK, &sigset, &orig_sigset);
}


void my_wait_loop(void)
{
pid_t pid;
int cstat, err;

for (;;)
{
while (!saw_sigchld)
{
sigsuspend(&orig_sigset);
}

saw_sigchld = 0;

lock_the_shared_datastructure();
do
{
pid = waitpid(-1, &cstat, WNOHANG);
} while (pid < 0 && (err = errno) == EINTR);
if (pid > 0)
{
handle_exited_child(pid);
}
else if (pid == 0 || err == ECHILD)
{
/* bogus SIGCHLD, just ignore it */
}
else
{
/* should not occur (EFAULT? EINVAL?) */
syslog("unexpected waitpid() error: %s", strerror(err));
}
unlock_the_shared_datastructure();
}
}


Your fork() code should call "sigprocmask(SIG_SETMASK, &orig_sigset,
NULL);" in the _child_, and if it isn't calling an exec-family
function then it should also reset SIGCHLD to SIG_DFL to avoid
possible conflicts with library calls.

Make sense?


Philip Guenther

[Brian Candler]

Post by Philip Guenther
Instead of separating the obtaining of the pid from the actual
reaping, you can instead separate the blocking from the return of the
pid+reaping. That lets you lock the datastructure only when you know
wait() won't block. To block until a child is ready to be reaped, use
SIGCHLD, blocking it when you aren't ready

Hmm, I hadn't thought of doing the actual reaping in a SIGCHLD handler, and
blocking SIGCHLD while doing the fork work. Or, as in your example, using
sigsuspend to wait for an instance of a SIGCHLD signal to occur, and only
then calling waitpid().

This then leaves me with a bunch of edge cases to satisfy myself are being

Post by Philip Guenther
void my_wait_loop(void)
{
pid_t pid;
int cstat, err;
for (;;)
{
while (!saw_sigchld)
{
sigsuspend(&orig_sigset);
}
saw_sigchld = 0;
lock_the_shared_datastructure();
do
{
pid = waitpid(-1, &cstat, WNOHANG);
} while (pid < 0 && (err = errno) == EINTR);
if (pid > 0)
{
handle_exited_child(pid);
}
else if (pid == 0 || err == ECHILD)
{
/* bogus SIGCHLD, just ignore it */
}
else
{
/* should not occur (EFAULT? EINVAL?) */
syslog("unexpected waitpid() error: %s", strerror(err));
}
unlock_the_shared_datastructure();
}
}

Suppose child 1 dies, causing a SIGCHLD to be pending, and then a second
child dies, before sigsuspend() unblocks the signal. sigsuspend returns, and
one child is reaped. Next time around the loop, will the second child be
reaped? If so, why?

I'm not saying that anything is actually wrong with the code you've
provided; rather, that it's difficult for me to understand the subtleties
involved in asynchronous signal-driven programming. And that's with a copy
of the Stevens book beside me :-)

Many thanks for giving me more food for thought.

Regards,

Brian.

[Darrin Chandler]

Post by Brian Candler
I'm not saying that anything is actually wrong with the code you've
provided; rather, that it's difficult for me to understand the subtleties
involved in asynchronous signal-driven programming. And that's with a copy
of the Stevens book beside me :-)

If you have the Stevens book (APUE, I assume), and you're already into
threads, reread the part talking about how to simplify your program
logic with threads. I don't have it here, so I can't give you a
chapter/section. But the idea is to put things in a thread so that your
calls become synchronous/blocking. I believe the APUE example was a
reader thread and a writer thread for a socket, with each using blocking
read/write calls. As long as you don't add more complexity from thread
sync, this can be a win.

[Brian Candler]

Post by Darrin Chandler

Post by Brian Candler
I'm not saying that anything is actually wrong with the code you've
provided; rather, that it's difficult for me to understand the subtleties
involved in asynchronous signal-driven programming. And that's with a copy
of the Stevens book beside me :-)

If you have the Stevens book (APUE, I assume), and you're already into
threads, reread the part talking about how to simplify your program
logic with threads. I don't have it here, so I can't give you a
chapter/section. But the idea is to put things in a thread so that your
calls become synchronous/blocking. I believe the APUE example was a
reader thread and a writer thread for a socket, with each using blocking
read/write calls. As long as you don't add more complexity from thread
sync, this can be a win.

Right book (although unfortunately I have only the first edition).

I think I'm trying to do exactly what you say:

- one thread blocks on waitpid() waiting for a child to die

- second thread does interesting work, and starts workers when it thinks
it's a good idea (e.g. system is getting "busy")

The issue, as I tried to describe in my original posting, is that if thread
one blocks on waitpid(), as soon as it returns with the PID and status of a
child, the second thread may fork a process with the same PID. I therefore
could end up having a data structure containing two children with the same
PID, one recently died (but not yet removed from the data structure), and
one recently forked. Or actually, my existing data structure could end up
losing track of one child.

The purpose of this post was trying to determine a good way to avoid that.

Regards,

Brian.

[Philip Guenther]

On 4/9/07, Brian Candler <***@pobox.com> wrote:
...

Post by Brian Candler
Suppose child 1 dies, causing a SIGCHLD to be pending, and then a second
child dies, before sigsuspend() unblocks the signal. sigsuspend returns, and
one child is reaped. Next time around the loop, will the second child be
reaped? If so, why?

Hmm, nice catch.

In theory, sigsuspend() should wake up immediately the second time
through, as SIGCHLD should be pending as long as there's a child in
the exited-but-not-yet-reaped state. That is, the first waitpid()
shouldn't cancel the pending state because there's another child
process ready to be reaped. To quote the SUS pages:
http://www.opengroup.org/onlinepubs/009695399/functions/waitpid.html
Otherwise, if SIGCHLD is blocked, if wait() or waitpid()
return because the status
of a child process is available, any pending SIGCHLD signal
shall be cleared
unless the status of another child process is available.

Note that last line.

However, OpenBSD 4.0 doesn't actually comply with that: after
waitpid() there will be no SIGCHLD pending, even if there are
additional children to reap.

So, if you're going to have multiple children, you need to call
waitpid(-1, &ret, WNOHANG) until it returns zero or -1/ECHILD before
you loop back to sigsuspend() again. That way you can be sure that
you haven't lost any SIGCHLDs before you reenter the sigsuspend().
I've actually confirmed that that loop does work as expected, unlike
the original example which only works with one child.


Philip Guenther

[Brian Candler]

Post by Philip Guenther
However, OpenBSD 4.0 doesn't actually comply with that: after
waitpid() there will be no SIGCHLD pending, even if there are
additional children to reap.
So, if you're going to have multiple children, you need to call
waitpid(-1, &ret, WNOHANG) until it returns zero or -1/ECHILD before
you loop back to sigsuspend() again. That way you can be sure that
you haven't lost any SIGCHLDs before you reenter the sigsuspend().
I've actually confirmed that that loop does work as expected, unlike
the original example which only works with one child.

Hmm. OK, thanks for that.

I think for now my preferred solution is to keep a linear list of child
processes. Forking adds a child to the end of the list; reaping finds the
first child in the list with a matching pid and removes that entry.

This eliminates the need for dealing with signals. The extra overhead of a
linear search is small, given that children don't die that often.

Cheers,

Brian.