From 29a144694d67645bac69251749103fdbbb67d9a7 Mon Sep 17 00:00:00 2001 From: hadashiA Date: Fri, 1 Sep 2023 19:52:22 +0900 Subject: [PATCH] Revert "Fixed https://github.com/Cysharp/UniTask/issues/444" --- .../Plugins/UniTask/Runtime/TriggerEvent.cs | 117 +++++++++++------- .../Assets/Tests/AsyncReactivePropertyTest.cs | 63 ---------- .../Tests/AsyncReactivePropertyTest.cs.meta | 3 - 3 files changed, 70 insertions(+), 113 deletions(-) delete mode 100644 src/UniTask/Assets/Tests/AsyncReactivePropertyTest.cs delete mode 100644 src/UniTask/Assets/Tests/AsyncReactivePropertyTest.cs.meta diff --git a/src/UniTask/Assets/Plugins/UniTask/Runtime/TriggerEvent.cs b/src/UniTask/Assets/Plugins/UniTask/Runtime/TriggerEvent.cs index 9f46b10..503e9db 100644 --- a/src/UniTask/Assets/Plugins/UniTask/Runtime/TriggerEvent.cs +++ b/src/UniTask/Assets/Plugins/UniTask/Runtime/TriggerEvent.cs @@ -20,6 +20,8 @@ namespace Cysharp.Threading.Tasks { ITriggerHandler head; // head.prev is last ITriggerHandler iteratingHead; + + bool preserveRemoveSelf; ITriggerHandler iteratingNode; void LogError(Exception ex) @@ -42,7 +44,6 @@ namespace Cysharp.Threading.Tasks while (h != null) { iteratingNode = h; - var next = h.Next; try { @@ -54,7 +55,18 @@ namespace Cysharp.Threading.Tasks Remove(h); } - h = next; + if (preserveRemoveSelf) + { + preserveRemoveSelf = false; + iteratingNode = null; + var next = h.Next; + Remove(h); + h = next; + } + else + { + h = h.Next; + } } iteratingNode = null; @@ -84,7 +96,8 @@ namespace Cysharp.Threading.Tasks { LogError(ex); } - + + preserveRemoveSelf = false; iteratingNode = null; var next = h.Next; Remove(h); @@ -118,7 +131,8 @@ namespace Cysharp.Threading.Tasks { LogError(ex); } - + + preserveRemoveSelf = false; iteratingNode = null; var next = h.Next; Remove(h); @@ -152,7 +166,8 @@ namespace Cysharp.Threading.Tasks { LogError(ex); } - + + preserveRemoveSelf = false; iteratingNode = null; var next = h.Next; Remove(h); @@ -225,64 +240,72 @@ namespace Cysharp.Threading.Tasks public void Remove(ITriggerHandler handler) { if (handler == null) throw new ArgumentNullException(nameof(handler)); - - var prev = handler.Prev; - var next = handler.Next; - if (next != null) + if (iteratingNode != null && iteratingNode == handler) { - next.Prev = prev; - } - - if (handler == head) - { - head = next; - } - else if (handler == iteratingHead) - { - iteratingHead = next; + // if remove self, reserve remove self after invoke completed. + preserveRemoveSelf = true; } else { - // when handler is head, prev indicate last so don't use it. - if (prev != null) - { - prev.Next = next; - } - } + var prev = handler.Prev; + var next = handler.Next; - if (head != null) - { - if (head.Prev == handler) + if (next != null) { - if (prev != head) + next.Prev = prev; + } + + if (handler == head) + { + head = next; + } + else if (handler == iteratingHead) + { + iteratingHead = next; + } + else + { + // when handler is head, prev indicate last so don't use it. + if (prev != null) { - head.Prev = prev; - } - else - { - head.Prev = null; + prev.Next = next; } } - } - if (iteratingHead != null) - { - if (iteratingHead.Prev == handler) + if (head != null) { - if (prev != iteratingHead.Prev) + if (head.Prev == handler) { - iteratingHead.Prev = prev; - } - else - { - iteratingHead.Prev = null; + if (prev != head) + { + head.Prev = prev; + } + else + { + head.Prev = null; + } } } - } - handler.Prev = null; - handler.Next = null; + if (iteratingHead != null) + { + if (iteratingHead.Prev == handler) + { + if (prev != iteratingHead.Prev) + { + iteratingHead.Prev = prev; + } + else + { + iteratingHead.Prev = null; + } + } + } + + handler.Prev = null; + handler.Next = null; + } } } } diff --git a/src/UniTask/Assets/Tests/AsyncReactivePropertyTest.cs b/src/UniTask/Assets/Tests/AsyncReactivePropertyTest.cs deleted file mode 100644 index f582443..0000000 --- a/src/UniTask/Assets/Tests/AsyncReactivePropertyTest.cs +++ /dev/null @@ -1,63 +0,0 @@ -using Cysharp.Threading.Tasks; -using FluentAssertions; -using System; -using System.Collections; -using System.Threading; -using UnityEngine.TestTools; - -namespace Cysharp.Threading.TasksTests -{ - public class AsyncReactivePropertyTest - { - private int _callCounter; - - [UnityTest] - public IEnumerator WaitCancelWait() => UniTask.ToCoroutine(async () => - { - // Test case for https://github.com/Cysharp/UniTask/issues/444 - - var property = new AsyncReactiveProperty(0); - - var cts1 = new CancellationTokenSource(); - var cts2 = new CancellationTokenSource(); - WaitForProperty(property, cts1.Token); - WaitForProperty(property, cts2.Token); - - _callCounter = 0; - property.Value = 1; - _callCounter.Should().Be(2); - - cts2.Cancel(); - cts2.Dispose(); - cts1.Cancel(); - cts1.Dispose(); - - var cts3 = new CancellationTokenSource(); - WaitForProperty(property, cts3.Token); - - _callCounter = 0; - property.Value = 2; - _callCounter.Should().Be(1); - - cts3.Cancel(); - cts3.Dispose(); - await UniTask.CompletedTask; - }); - - private async void WaitForProperty(AsyncReactiveProperty property, CancellationToken token) - { - while (true) - { - try - { - await property.WaitAsync(token); - _callCounter++; - } - catch (OperationCanceledException) - { - break; - } - } - } - } -} diff --git a/src/UniTask/Assets/Tests/AsyncReactivePropertyTest.cs.meta b/src/UniTask/Assets/Tests/AsyncReactivePropertyTest.cs.meta deleted file mode 100644 index 0c29ebf..0000000 --- a/src/UniTask/Assets/Tests/AsyncReactivePropertyTest.cs.meta +++ /dev/null @@ -1,3 +0,0 @@ -fileFormatVersion: 2 -guid: 27665955eefb4448969b8cc4dd204600 -timeCreated: 1676129650 \ No newline at end of file