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 register/unregister issues of WeakReferenceMessenger #4082

Merged
5 commits merged into from
Jul 5, 2021
Merged

Fix register/unregister issues of WeakReferenceMessenger #4082

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
04977e1
Fix register/unregister issues of WeakReferenceMessenger
steinhilber Jun 21, 2021
05d95f8
Update UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs
steinhilber Jun 23, 2021
928fc47
Update UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs
steinhilber Jun 23, 2021
0927e56
Merge branch 'main' into fix-weakreferencemessenger-bug
steinhilber Jun 25, 2021
805438c
Merge branch 'main' into fix-weakreferencemessenger-bug
steinhilber Jul 1, 2021
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
4 changes: 3 additions & 1 deletion Microsoft.Toolkit.Mvvm/Messaging/WeakReferenceMessenger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,8 @@ public bool MoveNext()

while (node is not null)
{
LinkedListNode<WeakReference<TKey>>? nextNode = node.Next;

// Get the key and value for the current node
if (node.Value.TryGetTarget(out TKey? target) &&
this.owner.table.TryGetValue(target!, out TValue? value))
Expand All @@ -421,7 +423,7 @@ public bool MoveNext()
this.owner.keys.Remove(node);
}

node = node.Next;
node = nextNode;
}

return false;
Expand Down
28 changes: 27 additions & 1 deletion UnitTests/UnitTests.Shared/Mvvm/Test_Messenger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,32 @@ void Test()
messenger.Cleanup();
}

// See https://github.com/windows-toolkit/WindowsCommunityToolkit/issues/4081
[TestCategory("Mvvm")]
[TestMethod]
[DataRow(typeof(StrongReferenceMessenger))]
[DataRow(typeof(WeakReferenceMessenger))]
public void Test_Messenger_RegisterMultiple_UnregisterSingle(Type type)
{
var messenger = (IMessenger)Activator.CreateInstance(type);

var recipient1 = new object();
var recipient2 = new object();

int handler1CalledCount = 0;
int handler2CalledCount = 0;

messenger.Register<object, MessageA>(recipient1, (r, m) => { handler1CalledCount++; });
messenger.Register<object, MessageA>(recipient2, (r, m) => { handler2CalledCount++; });

messenger.UnregisterAll(recipient2);

messenger.Send(new MessageA());

Assert.AreEqual(1, handler1CalledCount);
Assert.AreEqual(0, handler2CalledCount);
}

public sealed class RecipientWithNoMessages
{
public int Number { get; set; }
Expand Down Expand Up @@ -550,4 +576,4 @@ public sealed class MessageB
{
}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-hawker Just a note: this is the same issue you had raised as well while reviewing #3893: without some automated process to either fix the formatting or at least validate that the state is kept consistent, it's really really easy to reintroduce differences in whitespace formatting while making changes (especially because VS sometimes applies these changes automatically) and still end up with the same situation we had before. As in, the changes in #3893 aren't really that easily maintainable in the long term right now without still ending up with some inconsistencies here and there by accident 🤔