kernel/team: Fix race condition in team loading wait.
The condition variable that the load_image'ing thread is waiting on is also owned by that same thread, so as soon as it wakes up, it will soon return, thus destroying it. Under high load or other unlucky scheduling conditions, it seems this could occur before the other thread had even returned from the condition variable's NotifyAll. Since team->loading_info is protected by the team lock, simply acquire the team lock once more after being awoken and returning, to synchronize and prevent this race. Should fix #18352.
This commit is contained in:
@@ -426,20 +426,18 @@ notify_loading_app(status_t result, bool suspend)
|
||||
|
||||
TeamLocker teamLocker(team);
|
||||
|
||||
if (team->loading_info) {
|
||||
if (team->loading_info != NULL) {
|
||||
// there's indeed someone waiting
|
||||
struct team_loading_info* loadingInfo = team->loading_info;
|
||||
team->loading_info = NULL;
|
||||
|
||||
loadingInfo->result = result;
|
||||
|
||||
// we're done with the team stuff, get the scheduler lock instead
|
||||
teamLocker.Unlock();
|
||||
|
||||
thread_prepare_suspend();
|
||||
|
||||
// wake up the waiting thread
|
||||
loadingInfo->condition.NotifyAll();
|
||||
team->loading_info->result = result;
|
||||
team->loading_info->condition.NotifyAll();
|
||||
team->loading_info = NULL;
|
||||
|
||||
// we're done with the team stuff
|
||||
teamLocker.Unlock();
|
||||
|
||||
// suspend ourselves, if desired
|
||||
if (suspend)
|
||||
|
||||
@@ -1765,11 +1765,13 @@ load_image_internal(char**& _flatArgs, size_t flatArgsSize, int32 argCount,
|
||||
return B_NO_MEMORY;
|
||||
BReference<Team> teamReference(team, true);
|
||||
|
||||
BReference<Team> teamLoadingReference;
|
||||
if ((flags & B_WAIT_TILL_LOADED) != 0) {
|
||||
loadingInfo.condition.Init(team, "image load");
|
||||
loadingInfo.condition.Add(&loadingWaitEntry);
|
||||
loadingInfo.result = B_ERROR;
|
||||
team->loading_info = &loadingInfo;
|
||||
teamLoadingReference = teamReference;
|
||||
}
|
||||
|
||||
// get the parent team
|
||||
@@ -1892,6 +1894,14 @@ load_image_internal(char**& _flatArgs, size_t flatArgsSize, int32 argCount,
|
||||
// responsible for unsetting `loading_info` in the team structure.
|
||||
loadingWaitEntry.Wait();
|
||||
|
||||
// We must synchronize with the thread that woke us up, to ensure
|
||||
// there are no remaining consumers of the team_loading_info.
|
||||
team->Lock();
|
||||
if (team->loading_info != NULL)
|
||||
panic("team loading wait complete, but loading_info != NULL");
|
||||
team->Unlock();
|
||||
teamLoadingReference.Unset();
|
||||
|
||||
if (loadingInfo.result < B_OK)
|
||||
return loadingInfo.result;
|
||||
}
|
||||
@@ -3288,15 +3298,13 @@ team_delete_team(Team* team, port_id debuggerPort)
|
||||
|
||||
TeamLocker teamLocker(team);
|
||||
|
||||
if (team->loading_info) {
|
||||
if (team->loading_info != NULL) {
|
||||
// there's indeed someone waiting
|
||||
struct team_loading_info* loadingInfo = team->loading_info;
|
||||
team->loading_info = NULL;
|
||||
|
||||
loadingInfo->result = B_ERROR;
|
||||
team->loading_info->result = B_ERROR;
|
||||
|
||||
// wake up the waiting thread
|
||||
loadingInfo->condition.NotifyAll();
|
||||
team->loading_info->condition.NotifyAll();
|
||||
team->loading_info = NULL;
|
||||
}
|
||||
|
||||
// notify team watchers
|
||||
|
||||
Reference in New Issue
Block a user