Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TriggerEvent problem with iterate breaking on Remove when it has multiple handlers #487

Merged
merged 2 commits into from
Sep 8, 2023
Merged

Fix TriggerEvent problem with iterate breaking on Remove when it has multiple handlers #487

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2294063
Fix TriggerEvent problem with iterate breaking on Remove when it has …
hadashiA Sep 2, 2023
af2e49a
Fix TriggerEvent.SetCancel
hadashiA Sep 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 48 additions & 68 deletions src/UniTask/Assets/Plugins/UniTask/Runtime/TriggerEvent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ public struct TriggerEvent<T>
{
ITriggerHandler<T> head; // head.prev is last
ITriggerHandler<T> iteratingHead;

bool preserveRemoveSelf;
ITriggerHandler<T> iteratingNode;

void LogError(Exception ex)
Expand Down Expand Up @@ -55,18 +53,9 @@ public void SetResult(T value)
Remove(h);
}

if (preserveRemoveSelf)
{
preserveRemoveSelf = false;
iteratingNode = null;
var next = h.Next;
Remove(h);
h = next;
}
else
{
h = h.Next;
}
// If `h` itself is removed by OnNext, h.Next is null.
// Therefore, instead of looking at h.Next, the `iteratingNode` reference itself is replaced.
h = h == iteratingNode ? h.Next : iteratingNode;
}

iteratingNode = null;
Expand Down Expand Up @@ -97,9 +86,8 @@ public void SetCanceled(CancellationToken cancellationToken)
LogError(ex);
}

preserveRemoveSelf = false;
var next = h == iteratingNode ? h.Next : iteratingNode;
iteratingNode = null;
var next = h.Next;
Remove(h);
h = next;
}
Expand Down Expand Up @@ -132,9 +120,8 @@ public void SetCompleted()
LogError(ex);
}

preserveRemoveSelf = false;
var next = h == iteratingNode ? h.Next : iteratingNode;
iteratingNode = null;
var next = h.Next;
Remove(h);
h = next;
}
Expand Down Expand Up @@ -167,9 +154,8 @@ public void SetError(Exception exception)
LogError(ex);
}

preserveRemoveSelf = false;
var next = h == iteratingNode ? h.Next : iteratingNode;
iteratingNode = null;
var next = h.Next;
Remove(h);
h = next;
}
Expand Down Expand Up @@ -241,71 +227,65 @@ public void Remove(ITriggerHandler<T> handler)
{
if (handler == null) throw new ArgumentNullException(nameof(handler));

if (iteratingNode != null && iteratingNode == handler)
var prev = handler.Prev;
var next = handler.Next;

if (next != null)
{
// if remove self, reserve remove self after invoke completed.
preserveRemoveSelf = true;
next.Prev = prev;
}
else

if (handler == head)
{
head = next;
}
// when handler is head, prev indicate last so don't use it.
else if (prev != null)
{
var prev = handler.Prev;
var next = handler.Next;
prev.Next = next;
}

if (next != null)
{
next.Prev = prev;
}
if (handler == iteratingNode)
{
iteratingNode = next;
}
if (handler == iteratingHead)
{
iteratingHead = next;
}

if (handler == head)
{
head = next;
}
else if (handler == iteratingHead)
{
iteratingHead = next;
}
else
if (head != null)
{
if (head.Prev == handler)
{
// when handler is head, prev indicate last so don't use it.
if (prev != null)
if (prev != head)
{
prev.Next = next;
head.Prev = prev;
}
}

if (head != null)
{
if (head.Prev == handler)
else
{
if (prev != head)
{
head.Prev = prev;
}
else
{
head.Prev = null;
}
head.Prev = null;
}
}
}

if (iteratingHead != null)
if (iteratingHead != null)
{
if (iteratingHead.Prev == handler)
{
if (iteratingHead.Prev == handler)
if (prev != iteratingHead.Prev)
{
if (prev != iteratingHead.Prev)
{
iteratingHead.Prev = prev;
}
else
{
iteratingHead.Prev = null;
}
iteratingHead.Prev = prev;
}
else
{
iteratingHead.Prev = null;
}
}

handler.Prev = null;
handler.Next = null;
}

handler.Prev = null;
handler.Next = null;
}
}
}
Loading