Angus Leeming wrote:
> I do believe that we've got there. Attached is the final patch with
> a description of the design embedded in forkedcontr.C.
>
> I'll commit this tomorrow to give everybody a fair chance to
> complain loudly.
>
> Many thanks to John Levon for holding my hand through all this.
John, since you're on line...
I think I've found a race condition in this patch. Whether I have or
not depends on the semantics of sigprocmask. Perhaps you can set me
straight?
Consider:
std::vector<child_data> reaped_children;
sig_atomic_t current_child = -1;
extern "C"
void child_handler(int)
{
1 child_data & store = reaped_children[++current_child];
// Clean up the child process.
2 store.pid = wait(&store.status);
}
void ForkedcallsController::handleCompletedProcesses()
{
1 if (current_child == -1)
2 return;
// Block the SIGCHLD signal.
3 sigprocmask(SIG_BLOCK, &newMask, &oldMask);
4 for (int i = 0; i != 1+current_child; ++i) {
5 child_data & store = reaped_children[i];
// Go on to handle the child process
...
}
// Unblock the SIGCHLD signal and restore the old mask.
6 sigprocmask(SIG_SETMASK, &oldMask, 0);
}
The child_handler code is not atomic, so it is possible for the
execution to go:
chiild_handler line 1
handleCompletedProcesses line 3
handleCompletedProcesses line 4
At least, that's my understanding of sigprocmask. It prevents any new
signals being posted to child_handler. It does not wait for an
existing signal that has already been posted to child_handler being
processed.
Right?
If so, then we need a guard:
static sig_atomic_t child_guard = 0;
extern "C"
void child_handler(int)
{
1 child_guard = 1;
2 child_data & store = reaped_children[++current_child];
// Clean up the child process.
3 store.pid = wait(&store.status);
4 child_guard = 0;
}
void ForkedcallsController::handleCompletedProcesses()
{
1 if (current_child == -1)
2 return;
// Block the SIGCHLD signal.
3 sigprocmask(SIG_BLOCK, &newMask, &oldMask);
// Wait for an existing signal to finish being processed.
4 while (child_guard) {}
5 for (int i = 0; i != 1+current_child; ++i) {
...
Could you put me straight?
--
Angus
