Skip to content

Commit 3bd1362

Browse files
author
Stewart Miles
committed
Limited nesting in RunOnMainThread.
RunOnMainThread.Run was unneccessarily nesting task execution in both batch and interactive modes which - in the worst case - could lead to a stack overflow if a task is explicitly scheduled in the future to avoid recurision. Bug: 150471207 Change-Id: I3c4dc453e0a8fc2eca7c8a33c75237666a4496fb
1 parent c59bf5e commit 3bd1362

File tree

1 file changed

+41
-13
lines changed

1 file changed

+41
-13
lines changed

source/VersionHandlerImpl/src/RunOnMainThread.cs

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,11 @@ private static bool OnMainThread {
207207
get { return mainThreadId == System.Threading.Thread.CurrentThread.ManagedThreadId; }
208208
}
209209

210+
/// <summary>
211+
/// Set when the current thread is running ExecuteAll().
212+
/// </summary>
213+
private static bool runningExecuteAll = false;
214+
210215
/// <summary>
211216
/// Flag which indicates whether any jobs are running on the main thread.
212217
/// This is set and cleared by RunAction().
@@ -218,7 +223,7 @@ private static bool OnMainThread {
218223
/// This property is reset to its' default value after each set of jobs is dispatched.
219224
/// </summary>
220225
public static bool ExecuteNow {
221-
get { return ExecutionEnvironment.InBatchMode && !runningJobs; }
226+
get { return ExecutionEnvironment.InBatchMode && !runningJobs && !runningExecuteAll; }
222227
}
223228

224229
/// <summary>
@@ -302,7 +307,7 @@ public static void PollOnUpdateUntilComplete(Func<bool> condition, bool synchron
302307
if (ExecuteNow && OnMainThread) {
303308
RunAction(() => {
304309
while (true) {
305-
ExecuteAll();
310+
ExecuteAllUnnested(true);
306311
lock (pollingJobs) {
307312
if (pollingJobs.Count == 0) break;
308313
}
@@ -316,7 +321,7 @@ public static void PollOnUpdateUntilComplete(Func<bool> condition, bool synchron
316321
if (!pollingJobs.Contains(condition)) break;
317322
}
318323
if (OnMainThread) {
319-
ExecuteAll();
324+
ExecuteAllUnnested(true);
320325
} else {
321326
// Wait 100ms.
322327
Thread.Sleep(100);
@@ -409,7 +414,7 @@ public static void Run(Action job, bool runNow = true) {
409414
// If we're not executing the job right now, the job queue is pumped from the UnityEditor
410415
// update event.
411416
if (firstJob && (runNow || ExecuteNow) && OnMainThread) {
412-
ExecuteAll();
417+
ExecuteAllUnnested(false);
413418
}
414419
}
415420

@@ -437,7 +442,7 @@ private static bool ExecuteNext() {
437442
/// <returns>true if the caller is on the main thread, false otherwise.</returns>
438443
public static bool TryExecuteAll() {
439444
if (OnMainThread) {
440-
ExecuteAll();
445+
ExecuteAllUnnested(true);
441446
return true;
442447
}
443448
return false;
@@ -446,22 +451,45 @@ public static bool TryExecuteAll() {
446451
/// <summary>
447452
/// Execute all scheduled jobs and remove from the update loop if no jobs are remaining.
448453
/// </summary>
454+
/// <param name="force">Force execution when a re-entrant call of this method is detected.
455+
/// This is useful when an application is forcing execution to block the main thread.</param>
449456
private static void ExecuteAll() {
457+
ExecuteAllUnnested(false);
458+
}
459+
460+
/// <summary>
461+
/// Execute all scheduled jobs and remove from the update loop if no jobs are remaining.
462+
/// </summary>
463+
/// <param name="allowNested">Force execution when a re-entrant call of this method is detected.
464+
/// This is useful when an application is forcing execution to block the main thread.</param>
465+
private static void ExecuteAllUnnested(bool allowNested) {
450466
if (!OnMainThread) {
451467
UnityEngine.Debug.LogError("ExecuteAll must be executed from the main thread.");
452468
return;
453469
}
454470

471+
// Don't nest job execution on the main thread, return to the last stack frame
472+
// running ExecuteAll().
473+
if (runningExecuteAll && !allowNested) return;
474+
455475
RunAction(() => {
456-
// Execute jobs.
457-
while (ExecuteNext()) {
458-
}
476+
runningExecuteAll = true;
477+
bool jobsRemaining = true;
478+
while (jobsRemaining) {
479+
jobsRemaining = false;
480+
// Execute jobs.
481+
while (ExecuteNext()) {
482+
jobsRemaining = true;
483+
}
459484

460-
// Execute polling jobs.
461-
int remainingJobs;
462-
do {
463-
remainingJobs = ExecutePollingJobs();
464-
} while (remainingJobs > 0 && ExecutionEnvironment.InBatchMode);
485+
// Execute polling jobs.
486+
int remainingJobs = ExecutePollingJobs();
487+
// If we're in batch mode, keep on executing until no polling jobs remain.
488+
if (ExecutionEnvironment.InBatchMode && remainingJobs > 0) {
489+
jobsRemaining = true;
490+
}
491+
}
492+
runningExecuteAll = false;
465493
});
466494
}
467495
}

0 commit comments

Comments
 (0)