kernel: Fix race condition when waiting for load of new team.

There was no synchronization of the check of the done flag and the
waiting thread suspending to wait for it. It was therefore possible that
the new team both set the flag and triggered the wakeup of the waiting
thread in that time window, causing it to miss both the set flag and the
thread resumption.

Use a condition variable instead.

Fixes #13081.

Change-Id: I93c45db8dd773fe42b45c4b67153bcd39e200d3b
Reviewed-on: https://review.haiku-os.org/803
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
(cherry picked from commit 919185caba)
Reviewed-on: https://review.haiku-os.org/804
This commit is contained in:
Michael Lotz 2018-12-27 00:08:57 +01:00 committed by waddlesplash
parent b420dec40c
commit 25556bf64a
3 changed files with 12 additions and 16 deletions

View File

@ -83,9 +83,8 @@ struct thread_death_entry {
};
struct team_loading_info {
Thread* thread; // the waiting thread
ConditionVariable condition;
status_t result; // the result of the loading
bool done; // set when loading is done/aborted
};
struct team_watcher {

View File

@ -431,7 +431,6 @@ notify_loading_app(status_t result, bool suspend)
team->loading_info = NULL;
loadingInfo->result = result;
loadingInfo->done = true;
// we're done with the team stuff, get the scheduler lock instead
teamLocker.Unlock();
@ -439,7 +438,7 @@ notify_loading_app(status_t result, bool suspend)
thread_prepare_suspend();
// wake up the waiting thread
thread_continue(loadingInfo->thread);
loadingInfo->condition.NotifyAll();
// suspend ourselves, if desired
if (suspend)

View File

@ -1681,6 +1681,7 @@ load_image_internal(char**& _flatArgs, size_t flatArgsSize, int32 argCount,
status_t status;
struct team_arg* teamArgs;
struct team_loading_info loadingInfo;
ConditionVariableEntry loadingWaitEntry;
io_context* parentIOContext = NULL;
team_id teamID;
bool teamLimitReached = false;
@ -1713,10 +1714,10 @@ load_image_internal(char**& _flatArgs, size_t flatArgsSize, int32 argCount,
return B_NO_MEMORY;
BReference<Team> teamReference(team, true);
if (flags & B_WAIT_TILL_LOADED) {
loadingInfo.thread = thread_get_current_thread();
if ((flags & B_WAIT_TILL_LOADED) != 0) {
loadingInfo.condition.Init(team, "image load");
loadingInfo.condition.Add(&loadingWaitEntry);
loadingInfo.result = B_ERROR;
loadingInfo.done = false;
team->loading_info = &loadingInfo;
}
@ -1834,13 +1835,11 @@ load_image_internal(char**& _flatArgs, size_t flatArgsSize, int32 argCount,
thread_continue(mainThread);
}
// Now suspend ourselves until loading is finished. We will be woken
// either by the thread, when it finished or aborted loading, or when
// the team is going to die (e.g. is killed). In either case the one
// setting `loadingInfo.done' is responsible for removing the info from
// the team structure.
while (!loadingInfo.done)
thread_suspend();
// Now wait until loading is finished. We will be woken either by the
// thread, when it finished or aborted loading, or when the team is
// going to die (e.g. is killed). In either case the one notifying is
// responsible for unsetting `loading_info` in the team structure.
loadingWaitEntry.Wait();
if (loadingInfo.result < B_OK)
return loadingInfo.result;
@ -3229,10 +3228,9 @@ team_delete_team(Team* team, port_id debuggerPort)
team->loading_info = NULL;
loadingInfo->result = B_ERROR;
loadingInfo->done = true;
// wake up the waiting thread
thread_continue(loadingInfo->thread);
loadingInfo->condition.NotifyAll();
}
// notify team watchers