From 13beda00d3b2f67f45d9427396b9785145d07fb6 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Thu, 27 Dec 2018 00:08:57 +0100 Subject: [PATCH] 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 --- headers/private/kernel/thread_types.h | 3 +-- src/system/kernel/image.cpp | 3 +-- src/system/kernel/team.cpp | 22 ++++++++++------------ 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/headers/private/kernel/thread_types.h b/headers/private/kernel/thread_types.h index 33a535c708..1da7cd5c5d 100644 --- a/headers/private/kernel/thread_types.h +++ b/headers/private/kernel/thread_types.h @@ -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 { diff --git a/src/system/kernel/image.cpp b/src/system/kernel/image.cpp index 30c4b293b0..3fb9b72051 100644 --- a/src/system/kernel/image.cpp +++ b/src/system/kernel/image.cpp @@ -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) diff --git a/src/system/kernel/team.cpp b/src/system/kernel/team.cpp index 8fa4cb36e5..d4ff25fbc6 100644 --- a/src/system/kernel/team.cpp +++ b/src/system/kernel/team.cpp @@ -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 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