diff --git a/headers/private/kernel/thread_types.h b/headers/private/kernel/thread_types.h index a82f0cba86..5ada250358 100644 --- a/headers/private/kernel/thread_types.h +++ b/headers/private/kernel/thread_types.h @@ -66,6 +66,7 @@ struct process_group { pid_t id; sem_id dead_child_sem; int32 wait_for_any; + int32 dead_child_waiters; // count of threads waiting for sem struct team *teams; }; @@ -96,16 +97,17 @@ struct team { struct process_group *group; char name[B_OS_NAME_LENGTH]; char args[64]; // contents for the team_info::args field - int num_threads; /* number of threads in this team */ - int state; /* current team state, see above */ + int num_threads; // number of threads in this team + int state; // current team state, see above int pending_signals; void *io_context; - sem_id death_sem; /* semaphore to wait on for dying threads */ + sem_id death_sem; // semaphore to wait on for dying threads struct { - sem_id sem; /* wait for dead child entries */ + sem_id sem; // wait for dead child entries struct list list; uint32 count; - int32 wait_for_any; /* count of wait_for_child() that wait for any child */ + vint32 wait_for_any; // count of wait_for_child() that wait for any child + int32 waiters; // count of all threads waiting for dead children sem bigtime_t kernel_time; bigtime_t user_time; } dead_children; diff --git a/src/system/kernel/team.c b/src/system/kernel/team.c index 3df96721cc..a7302b483f 100644 --- a/src/system/kernel/team.c +++ b/src/system/kernel/team.c @@ -618,6 +618,7 @@ create_process_group(pid_t id) group->session = NULL; group->teams = NULL; group->wait_for_any = 0; + group->dead_child_waiters++; return group; } @@ -720,6 +721,7 @@ create_team_struct(const char *name, bool kernel) list_init(&team->dead_children.list); team->dead_children.count = 0; team->dead_children.wait_for_any = 0; + team->dead_children.waiters = 0; team->dead_children.kernel_time = 0; team->dead_children.user_time = 0; team->dead_children.sem = create_sem(0, "dead children"); @@ -1628,7 +1630,7 @@ get_team_death_entry(struct team *team, thread_id child, struct death_entry *dea static status_t get_death_entry(struct team *team, pid_t child, struct death_entry *death, - sem_id *_waitSem, struct death_entry **_freeDeath) + sem_id *_waitSem, int32 **_waitCount, struct death_entry **_freeDeath) { struct process_group *group; status_t status; @@ -1636,6 +1638,7 @@ get_death_entry(struct team *team, pid_t child, struct death_entry *death, if (child == -1 || child > 0) { // wait for any children or a specific child of this team to die *_waitSem = team->dead_children.sem; + *_waitCount = &team->dead_children.waiters; return get_team_death_entry(team, child, death, _freeDeath); } else if (child < 0) { // we wait for all children of the specified process group @@ -1656,6 +1659,7 @@ get_death_entry(struct team *team, pid_t child, struct death_entry *death, } *_waitSem = group->dead_child_sem; + *_waitCount = &group->dead_child_waiters; return B_WOULD_BLOCK; } @@ -1670,8 +1674,6 @@ wait_for_child(thread_id child, uint32 flags, int32 *_reason, status_t *_returnC struct team *team = thread_get_current_thread()->team; struct death_entry death, *freeDeath = NULL; status_t status = B_OK; - sem_id waitSem; - cpu_status state; bool childExists = false; TRACE(("wait_for_child(child = %ld, flags = %ld)\n", child, flags)); @@ -1689,7 +1691,10 @@ wait_for_child(thread_id child, uint32 flags, int32 *_reason, status_t *_returnC } while (true) { - state = disable_interrupts(); + int32 *waitCount; + sem_id waitSem; + + cpu_status state = disable_interrupts(); GRAB_THREAD_LOCK(); if (child > 0 && !childExists) { @@ -1720,7 +1725,23 @@ wait_for_child(thread_id child, uint32 flags, int32 *_reason, status_t *_returnC GRAB_TEAM_LOCK(); - status = get_death_entry(team, child, &death, &waitSem, &freeDeath); + status = get_death_entry(team, child, &death, &waitSem, &waitCount, &freeDeath); + + // there was no matching group/child we could wait for + if (status == B_BAD_THREAD_ID) { + if (child <= 0 || !childExists) { + status = ECHILD; + goto err; + } else { + // the specific child we're waiting for is still running + status = B_WOULD_BLOCK; + } + } + if (status == B_WOULD_BLOCK && (flags & WNOHANG) == 0) { + // We need to hold the team lock when changing this counter, + // but of course only if we really will wait later + (*waitCount)++; + } RELEASE_TEAM_LOCK(); restore_interrupts(state); @@ -1728,16 +1749,8 @@ wait_for_child(thread_id child, uint32 flags, int32 *_reason, status_t *_returnC // we got our death entry and can return to our caller if (status == B_OK) break; - - // there was no matching group/child we could wait for - if (status == B_BAD_THREAD_ID) { - if (child <= 0 || !childExists) { - status = ECHILD; - goto err; - } - - // the specific child we're waiting for is still running - } + if (status != B_WOULD_BLOCK) + goto err; if ((flags & WNOHANG) != 0) { status = B_WOULD_BLOCK; diff --git a/src/system/kernel/thread.c b/src/system/kernel/thread.c index f592956f9c..e35ad2971d 100644 --- a/src/system/kernel/thread.c +++ b/src/system/kernel/thread.c @@ -1008,11 +1008,13 @@ thread_exit(void) // notify listeners that a new death entry is available // TODO: should that be moved to handle_signal() (for SIGCHLD)? - // TODO: B_RELEASE_ALL opens up a race condition with wait_for_child(). - release_sem_etc(parent->dead_children.sem, 0, - B_RELEASE_ALL | B_DO_NOT_RESCHEDULE); - release_sem_etc(team->group->dead_child_sem, 0, - B_RELEASE_ALL | B_DO_NOT_RESCHEDULE); + release_sem_etc(parent->dead_children.sem, + parent->dead_children.waiters, B_DO_NOT_RESCHEDULE); + release_sem_etc(team->group->dead_child_sem, + team->group->dead_child_waiters, B_DO_NOT_RESCHEDULE); + + parent->dead_children.waiters = 0; + team->group->dead_child_waiters = 0; } team_remove_team(team, &freeGroup);