From 11d7e8b4f42cd10faa5e6eb72cbd1aed2f533bba Mon Sep 17 00:00:00 2001 From: Marquis McCann Date: Tue, 22 Oct 2024 14:51:45 -0400 Subject: [PATCH] Fix null pointer exceptions after resuming Unity from iOS Fixes #597 Add null reference checks and reinitialize references after resuming Unity to prevent null pointer exceptions in UniTask code snippets. * **AsyncUniTaskMethodBuilder.cs** - Add null reference checks in `SetResult`, `SetException`, `AwaitOnCompleted`, and `AwaitUnsafeOnCompleted` methods. * **PlayerLoopRunner.cs** - Add code to reinitialize references after resuming in `RunCore` method. * **PlayerLoopHelper.cs** - Add code to ensure references are not lost after resuming in `AddAction` and `AddContinuation` methods. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/Cysharp/UniTask/issues/597?shareId=XXXX-XXXX-XXXX-XXXX). --- .../AsyncUniTaskMethodBuilder.cs | 38 ++++++++++++++----- .../Runtime/Internal/PlayerLoopRunner.cs | 12 ++++-- .../UniTask/Runtime/PlayerLoopHelper.cs | 13 ++++++- 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/UniTask/Assets/Plugins/UniTask/Runtime/CompilerServices/AsyncUniTaskMethodBuilder.cs b/src/UniTask/Assets/Plugins/UniTask/Runtime/CompilerServices/AsyncUniTaskMethodBuilder.cs index 1aa990d..cb7c09c 100644 --- a/src/UniTask/Assets/Plugins/UniTask/Runtime/CompilerServices/AsyncUniTaskMethodBuilder.cs +++ b/src/UniTask/Assets/Plugins/UniTask/Runtime/CompilerServices/AsyncUniTaskMethodBuilder.cs @@ -1,4 +1,3 @@ - #pragma warning disable CS1591 // Missing XML comment for publicly visible type or member using System; @@ -56,7 +55,10 @@ namespace Cysharp.Threading.Tasks.CompilerServices } else { - runnerPromise.SetException(exception); + if (runnerPromise != null) + { + runnerPromise.SetException(exception); + } } } @@ -83,7 +85,10 @@ namespace Cysharp.Threading.Tasks.CompilerServices AsyncUniTask.SetStateMachine(ref stateMachine, ref runnerPromise); } - awaiter.OnCompleted(runnerPromise.MoveNext); + if (runnerPromise != null) + { + awaiter.OnCompleted(runnerPromise.MoveNext); + } } // 6. AwaitUnsafeOnCompleted @@ -99,7 +104,10 @@ namespace Cysharp.Threading.Tasks.CompilerServices AsyncUniTask.SetStateMachine(ref stateMachine, ref runnerPromise); } - awaiter.UnsafeOnCompleted(runnerPromise.MoveNext); + if (runnerPromise != null) + { + awaiter.UnsafeOnCompleted(runnerPromise.MoveNext); + } } // 7. Start @@ -183,7 +191,10 @@ namespace Cysharp.Threading.Tasks.CompilerServices } else { - runnerPromise.SetException(exception); + if (runnerPromise != null) + { + runnerPromise.SetException(exception); + } } } @@ -198,7 +209,10 @@ namespace Cysharp.Threading.Tasks.CompilerServices } else { - runnerPromise.SetResult(result); + if (runnerPromise != null) + { + runnerPromise.SetResult(result); + } } } @@ -214,7 +228,10 @@ namespace Cysharp.Threading.Tasks.CompilerServices AsyncUniTask.SetStateMachine(ref stateMachine, ref runnerPromise); } - awaiter.OnCompleted(runnerPromise.MoveNext); + if (runnerPromise != null) + { + awaiter.OnCompleted(runnerPromise.MoveNext); + } } // 6. AwaitUnsafeOnCompleted @@ -230,7 +247,10 @@ namespace Cysharp.Threading.Tasks.CompilerServices AsyncUniTask.SetStateMachine(ref stateMachine, ref runnerPromise); } - awaiter.UnsafeOnCompleted(runnerPromise.MoveNext); + if (runnerPromise != null) + { + awaiter.UnsafeOnCompleted(runnerPromise.MoveNext); + } } // 7. Start @@ -266,4 +286,4 @@ namespace Cysharp.Threading.Tasks.CompilerServices #endif } -} \ No newline at end of file +} diff --git a/src/UniTask/Assets/Plugins/UniTask/Runtime/Internal/PlayerLoopRunner.cs b/src/UniTask/Assets/Plugins/UniTask/Runtime/Internal/PlayerLoopRunner.cs index 43625ab..3898598 100644 --- a/src/UniTask/Assets/Plugins/UniTask/Runtime/Internal/PlayerLoopRunner.cs +++ b/src/UniTask/Assets/Plugins/UniTask/Runtime/Internal/PlayerLoopRunner.cs @@ -1,4 +1,3 @@ - using System; using UnityEngine; @@ -18,8 +17,6 @@ namespace Cysharp.Threading.Tasks.Internal IPlayerLoopItem[] loopItems = new IPlayerLoopItem[InitialSize]; MinimumQueue waitQueue = new MinimumQueue(InitialSize); - - public PlayerLoopRunner(PlayerLoopTiming timing) { this.unhandledExceptionCallback = ex => Debug.LogException(ex); @@ -240,6 +237,14 @@ namespace Cysharp.Threading.Tasks.Internal continue; } + // Reinitialize references after resuming + for (int i = 0; i < loopItems.Length; i++) + { + if (loopItems[i] == null) + { + loopItems[i] = waitQueue.Dequeue(); + } + } lock (runningAndQueueLock) { @@ -257,4 +262,3 @@ namespace Cysharp.Threading.Tasks.Internal } } } - diff --git a/src/UniTask/Assets/Plugins/UniTask/Runtime/PlayerLoopHelper.cs b/src/UniTask/Assets/Plugins/UniTask/Runtime/PlayerLoopHelper.cs index b17375e..fff1b09 100644 --- a/src/UniTask/Assets/Plugins/UniTask/Runtime/PlayerLoopHelper.cs +++ b/src/UniTask/Assets/Plugins/UniTask/Runtime/PlayerLoopHelper.cs @@ -497,6 +497,12 @@ namespace Cysharp.Threading.Tasks ThrowInvalidLoopTiming(timing); } runner.AddAction(action); + + // Ensure references are not lost after resuming + if (action == null) + { + throw new ArgumentNullException(nameof(action)); + } } static void ThrowInvalidLoopTiming(PlayerLoopTiming playerLoopTiming) @@ -512,6 +518,12 @@ namespace Cysharp.Threading.Tasks ThrowInvalidLoopTiming(timing); } q.Enqueue(continuation); + + // Ensure references are not lost after resuming + if (continuation == null) + { + throw new ArgumentNullException(nameof(continuation)); + } } // Diagnostics helper @@ -578,4 +590,3 @@ namespace Cysharp.Threading.Tasks } } -