Re: waitpid() thread race
On Mon, Apr 09, 2007 at 03:42:50PM -0600, Philip Guenther wrote: > 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.
Re: waitpid() thread race
On Mon, Apr 09, 2007 at 01:40:06PM -0700, Darrin Chandler wrote: > On Mon, Apr 09, 2007 at 09:10:39PM +0100, Brian Candler wrote: > > 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.
Re: waitpid() thread race
On 4/9/07, Brian Candler <[EMAIL PROTECTED]> wrote: ... 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
Re: waitpid() thread race
On Mon, Apr 09, 2007 at 09:10:39PM +0100, Brian Candler wrote: > 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. -- Darrin Chandler| Phoenix BSD User Group | MetaBUG [EMAIL PROTECTED] | http://phxbug.org/ | http://metabug.org/ http://www.stilyagin.com/ | Daemons in the Desert | Global BUG Federation
Re: waitpid() thread race
On Sat, Apr 07, 2007 at 11:09:55AM -0600, Philip Guenther wrote:
> 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
handled correctly. For example, in your my_wait_loop():
> 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.
Re: waitpid() thread race
On 4/7/07, Brian Candler <[EMAIL PROTECTED]> wrote:
...
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
Now, it seems to me there is a race condition here: between wait() returning
and the entry being removed from the data structure, the main program
may have forked off another child with the same
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.
...
(2) Modify the data structure <...>
...
(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:
...
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
Re: waitpid() thread race
On Sat, Apr 07, 2007 at 12:15:07PM +0100, Brian Candler wrote:
> 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
>
>
> Now, it seems to me there is a race condition here: between wait() returning
> and the entry being removed from the data structure, the main program
> may have forked off another child with the same
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.
> 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| Phoenix BSD User Group | MetaBUG
[EMAIL PROTECTED] | http://phxbug.org/ | http://metabug.org/
http://www.stilyagin.com/ | Daemons in the Desert | Global BUG Federation
