浏览代码

Fixing a bug with Messenger which allowed subscribers to add/remove subscribers during OnReceiveMessage that would themselves send a message, pushing through the add/remove before the original OnReceiveMessage call completed. This would cause a list mutation error since it happened within a foreach loop.

/main/staging/messenger_fix
nathaniel.buck@unity3d.com 3 年前
当前提交
2f4526c7
共有 2 个文件被更改,包括 65 次插入2 次删除
  1. 13
      Assets/Scripts/Infrastructure/Messenger.cs
  2. 54
      Assets/Scripts/Tests/Editor/MessengerTests.cs

13
Assets/Scripts/Infrastructure/Messenger.cs


// We need to handle subscribers who modify the receiver list, e.g. a subscriber who unsubscribes in their OnReceiveMessage.
private Queue<Action> m_pendingReceivers = new Queue<Action>();
private int m_recurseCount = 0;
/// <summary>
/// Assume that you won't receive messages in a specific order.

/// <param name="msg">If there's some data relevant to the recipient, include it here.</param>
public virtual void OnReceiveMessage(MessageType type, object msg)
{
while (m_pendingReceivers.Count > 0)
m_pendingReceivers.Dequeue()?.Invoke();
if (m_recurseCount > 5)
{ Debug.LogError("OnReceiveMessage recursion detected! Is something calling OnReceiveMessage when it receives a message?");
return;
}
if (m_recurseCount == 0) // This will increment if a new or existing subscriber calls OnReceiveMessage while handling a message. This is expected occasionally but shouldn't go too deep.
while (m_pendingReceivers.Count > 0)
m_pendingReceivers.Dequeue()?.Invoke();
m_recurseCount++;
Stopwatch stopwatch = new Stopwatch();
foreach (IReceiveMessages receiver in m_receivers)
{

if (stopwatch.ElapsedMilliseconds > k_durationToleranceMs)
Debug.LogWarning($"Message recipient \"{receiver}\" took too long to process message \"{msg}\" of type {type}");
}
m_recurseCount--;
}
public void OnReProvided(IMessenger previousProvider)

54
Assets/Scripts/Tests/Editor/MessengerTests.cs


}
/// <summary>
/// If a subscriber is added/removed during an OnReceiveMessage call and it immediately sends its own message, we must ensure that we don't process the change in the subscriber list.
/// (During a foreach loop, this is the "Collection was modified" error.)
/// </summary>
[Test]
[Timeout(100)] // These recursion tests could recurse infinitely, so time out (in ms) just in case. (We should see a StackOverflowException before then, though.)
public void RecurseWithinOnReceiveMessageDontModifyCollection()
{
Messenger messenger = new Messenger();
int msgCount = 0;
SubscriberArgs sub = new SubscriberArgs(Recurse);
messenger.Subscribe(sub);
LogAssert.Expect(UnityEngine.LogType.Error, new Regex(".*OnReceiveMessage recursion detected.*"));
messenger.OnReceiveMessage(MessageType.None, null);
Assert.AreEqual(6, msgCount, "Expected to break out of recursion and not allow subscriber list modification while recursing.");
LogAssert.ignoreFailingMessages = true; // The "recursion detected" error will appear multiple times.
messenger.OnReceiveMessage(MessageType.None, null);
LogAssert.ignoreFailingMessages = false;
Assert.Less(12, msgCount, "Subscribers added during recursion should be given messages afterward.");
void Recurse(MessageType type, object msg)
{
msgCount++;
if (type == MessageType.None) // This is just to prevent an exponential explosion of new subscribers. The exact type used is arbitrary.
{ SubscriberArgs newSub = new SubscriberArgs(Recurse);
messenger.Subscribe(newSub);
}
messenger.OnReceiveMessage(MessageType.RenameRequest, null);
}
}
[Test]
[Timeout(100)]
public void RecurseWithinOnReceiveMessageBreakout()
{
Messenger messenger = new Messenger();
int msgCount = 0;
Subscriber sub = new Subscriber(Recurse);
messenger.Subscribe(sub);
messenger.OnReceiveMessage(MessageType.None, null);
Assert.Pass("Should have broken out of a recursion loop if too deep.");
void Recurse()
{
msgCount++;
Subscriber newSub = new Subscriber(Recurse);
messenger.Subscribe(newSub);
messenger.OnReceiveMessage(MessageType.None, null);
}
}
/// <summary>
/// If a message recipient takes a long time to process a message, we want to be made aware.
/// </summary>
[Test]

正在加载...
取消
保存