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 <[email protected]>

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 {

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)

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<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