Skip to content

Commit

Permalink
Merge pull request #248 from timcassell/fix-promiseyielder
Browse files Browse the repository at this point in the history
Fix `IndexOutOfRangeException` in `PromiseYielder`
  • Loading branch information
timcassell authored Jun 13, 2023
2 parents de18e3c + 336e432 commit df39241
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 11 deletions.
97 changes: 97 additions & 0 deletions Package/Tests/UnityTests/PromiseYielderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,103 @@ public IEnumerator PromiseYielderWaitForFixedUpdate_CompletesInFixedUpdate()
// Not testing WaitForEndOfFrame as there is no way to assert that it is actually in that execution stage.
// Not testing WaitForAsyncOperation as I don't want to have to load something for unit testing.

[UnityTest]
public IEnumerator PromiseYielder_ManyEarlyWaitUntils()
{
// Use the testBehaviour to start the waits from Update so that it is ran early (before the PromiseYielder processes the queues).
var testBehaviour = behaviour.gameObject.AddComponent<WaitOneFrameTestBehaviour>();
try
{
var promise = testBehaviour.WaitOneFrameFromUpdate(() =>
{
// Wait for the internal queues to be processed twice to make sure nothing breaks.
const int waitFrameCount = 6;
// Testing implementation detail of the internal array growing in size. Initial size is 64, so we need to create at least 65 waits.
const int waitCount = 100;
int initialFrame = Time.frameCount;
Promise[] promises = new Promise[waitCount];
for (int i = 0; i < waitCount; i++)
{
promises[i] = PromiseYielder.WaitUntil(() => Time.frameCount >= initialFrame + waitFrameCount).ToPromise();
}
return Promise.All(promises);
});
using (var yieldInstruction = promise.ToYieldInstruction())
{
yield return yieldInstruction;
yieldInstruction.GetResult();
}
}
finally
{
UnityEngine.Object.Destroy(testBehaviour);
}
}

[UnityTest]
public IEnumerator PromiseYielder_ManyLateWaitUntils()
{
if (Application.isBatchMode)
{
Assert.Inconclusive("Application is running in batchmode, WaitForEndOfFrame will not run.");
yield break;
}

// Use the testBehaviour to start the waits from EndOfFrame so that it is ran early (before the PromiseYielder processes the queues).
var testBehaviour = behaviour.gameObject.AddComponent<WaitOneFrameTestBehaviour>();
try
{
var promise = testBehaviour.WaitOneFrameFromEndOfFrame(() =>
{
// Wait for the internal queues to be processed twice to make sure nothing breaks.
const int waitFrameCount = 6;
// Testing implementation detail of the internal array growing in size. Initial size is 64, so we need to create at least 65 waits.
const int waitCount = 100;
int initialFrame = Time.frameCount;
Promise[] promises = new Promise[waitCount];
for (int i = 0; i < waitCount; i++)
{
// Use capture value to make sure the internal queue is not the same queue used as PromiseYielder_ManyEarlyWaitUntils().
promises[i] = PromiseYielder.WaitUntil(initialFrame, iFrame => Time.frameCount >= iFrame + waitFrameCount).ToPromise();
}
return Promise.All(promises);
});
using (var yieldInstruction = promise.ToYieldInstruction())
{
yield return yieldInstruction;
yieldInstruction.GetResult();
}
}
finally
{
UnityEngine.Object.Destroy(testBehaviour);
}
}

[UnityTest]
public IEnumerator PromiseYielder_ManyWaitWhiles()
{
// Wait for the internal queues to be processed twice to make sure nothing breaks.
const int waitFrameCount = 6;
// Testing implementation detail of the internal array growing in size. Initial size is 64, so we need to create at least 65 waits.
const int waitCount = 100;

int initialFrame = Time.frameCount;
Promise[] promises = new Promise[waitCount];
for (int i = 0; i < waitCount; i++)
{
promises[i] = PromiseYielder.WaitWhile(() => Time.frameCount < initialFrame + waitFrameCount).ToPromise();
}
var promise = Promise.All(promises);
using (var yieldInstruction = promise.ToYieldInstruction())
{
yield return yieldInstruction;
yieldInstruction.GetResult();
}
}

#if PROMISE_PROGRESS
[UnityTest]
public IEnumerator PromiseYielderWaitForFrames_ReportsProgress()
Expand Down
30 changes: 19 additions & 11 deletions Package/UnityHelpers/Internal/PromiseYielderInternal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,10 @@ internal void WaitForNext(Action continuation)
int capacity = _nextQueue.Length;
if (index >= capacity)
{
// We only resize the array that is currently being added to. We don't resize the other arrays until they need it.
Array.Resize(ref _nextQueue, capacity * 2);
int newCapcity = capacity * 2;
Array.Resize(ref _currentQueue, newCapcity);
Array.Resize(ref _nextQueue, newCapcity);
Array.Resize(ref _followingQueue, newCapcity);
}

_nextQueue[index] = continuation;
Expand All @@ -177,8 +179,10 @@ internal void WaitForFollowing(Action continuation)
int capacity = _followingQueue.Length;
if (index >= capacity)
{
// We only resize the array that is currently being added to. We don't resize the other arrays until they need it.
Array.Resize(ref _followingQueue, capacity * 2);
int newCapcity = capacity * 2;
Array.Resize(ref _currentQueue, newCapcity);
Array.Resize(ref _nextQueue, newCapcity);
Array.Resize(ref _followingQueue, newCapcity);
}

_followingQueue[index] = continuation;
Expand All @@ -202,7 +206,7 @@ internal void Process()
{
current[i].Invoke();
}
Array.Clear(current, 0, max);
Array.Clear(_currentQueue, 0, max);
}
}

Expand Down Expand Up @@ -302,8 +306,10 @@ internal void WaitFor(ref TYieldInstruction instruction)
capacity = _followingQueue.Length;
if (_followingCount >= capacity)
{
// We only resize the array that is currently being added to. We don't resize the other one until it's needed.
Array.Resize(ref _nextQueue, capacity * 2);
int newCapcity = capacity * 2;
Array.Resize(ref _currentQueue, newCapcity);
Array.Resize(ref _nextQueue, newCapcity);
Array.Resize(ref _followingQueue, newCapcity);
}

_followingQueue[_followingCount] = instruction;
Expand All @@ -320,8 +326,10 @@ internal void WaitFor(ref TYieldInstruction instruction)
capacity = _nextQueue.Length;
if (potentialFutureCount >= capacity)
{
// We only resize the array that is currently being added to. We don't resize the other one until it's needed.
Array.Resize(ref _nextQueue, capacity * 2);
int newCapcity = capacity * 2;
Array.Resize(ref _currentQueue, newCapcity);
Array.Resize(ref _nextQueue, newCapcity);
Array.Resize(ref _followingQueue, newCapcity);
}

_nextQueue[_nextCount] = instruction;
Expand Down Expand Up @@ -349,15 +357,15 @@ public void Process()
Evaluate(ref current[i]);
--_currentCount;
}
Array.Clear(current, 0, max);
Array.Clear(_currentQueue, 0, max);
}

[MethodImpl(Internal.InlineOption)]
private void Evaluate(ref TYieldInstruction instruction)
{
if (!instruction.Evaluate())
{
// This is hottest path, so we don't do a bounds check here (see Add).
// This is hottest path, so we don't do a bounds check here (see WaitFor).
_nextQueue[_nextCount] = instruction;
++_nextCount;
}
Expand Down

0 comments on commit df39241

Please sign in to comment.