charleslemaux/Mirror
1
0
Fork 0
mirror of https://github.com/MirrorNetworking/Mirror.git synced 2024-11-18 02:50:32 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix: NetworkServer/NetworkClient batch processing now detects ever growing batches (as caused by #2882)

Browse Source
This commit is contained in:
vis2k 2021-09-02 17:54:27 +08:00
parent 9578deeb55
commit 3e4def126f
4 changed files with 41 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Assets/Mirror/Runtime/Batching/Unbatcher.cs
View File

@ -16,6 +16,8 @@ public class Unbatcher
// just in case.
Queue<PooledNetworkWriter> batches = new Queue<PooledNetworkWriter>();
public int BatchesCount => batches.Count;
// NetworkReader is only created once,
// then pointed to the first batch.
NetworkReader reader = new NetworkReader(new byte[0]);

18
Assets/Mirror/Runtime/NetworkClient.cs
View File

@ -364,6 +364,24 @@ internal static void OnTransportData(ArraySegment<byte> data, int channelId)
return;
}
}
// if we weren't interrupted by a scene change,
// then all batched messages should have been processed now.
// if not, we need to log an error to avoid debugging hell.
// otherwise batches would silently grow.
// we need to log an error to avoid debugging hell.
//
// EXAMPLE: https://github.com/vis2k/Mirror/issues/2882
// -> UnpackAndInvoke silently returned because no handler for id
// -> Reader would never be read past the end
// -> Batch would never be retired because end is never reached
//
// IMPORTANT: always keep this check to detect memory leaks.
// this took half a day to debug last time.
if (!isLoadingScene && unbatcher.BatchesCount > 0)
{
Debug.LogError($"Still had {unbatcher.BatchesCount} batches remaining after processing, even though processing was not interrupted by a scene change. This should never happen, as it would cause ever growing batches.");
}
}
else Debug.LogError("Skipped Data message handling because connection is null.");
}

17
Assets/Mirror/Runtime/NetworkServer.cs
View File

@ -497,6 +497,23 @@ internal static void OnTransportData(int connectionId, ArraySegment<byte> data,
return;
}
}
// if we weren't interrupted by a scene change,
// then all batched messages should have been processed now.
// otherwise batches would silently grow.
// we need to log an error to avoid debugging hell.
//
// EXAMPLE: https://github.com/vis2k/Mirror/issues/2882
// -> UnpackAndInvoke silently returned because no handler for id
// -> Reader would never be read past the end
// -> Batch would never be retired because end is never reached
//
// IMPORTANT: always keep this check to detect memory leaks.
// this took half a day to debug last time.
if (!isLoadingScene && connection.unbatcher.BatchesCount > 0)
{
Debug.LogError($"Still had {connection.unbatcher.BatchesCount} batches remaining after processing, even though processing was not interrupted by a scene change. This should never happen, as it would cause ever growing batches.");
}
}
else Debug.LogError("HandleData Unknown connectionId:" + connectionId);
}

4
Assets/Mirror/Tests/Editor/NetworkServerTest.cs
View File

@ -864,7 +864,9 @@ public void UnregisterHandler()
Assert.That(variant1Called, Is.EqualTo(1));
// unregister, send again, should not be called again
// error about remaining batches is expected
NetworkServer.UnregisterHandler<TestMessage1>();
LogAssert.Expect(LogType.Error, new Regex("Still had 1 batches remaining after processing.*"));
NetworkClient.Send(new TestMessage1());
ProcessMessages();
Assert.That(variant1Called, Is.EqualTo(1));
@ -887,7 +889,9 @@ public void ClearHandler()
Assert.That(variant1Called, Is.EqualTo(1));
// clear handlers, send again, should not be called again
// error about remaining batches is expected
NetworkServer.ClearHandlers();
LogAssert.Expect(LogType.Error, new Regex("Still had 1 batches remaining after processing.*"));
NetworkClient.Send(new TestMessage1());
ProcessMessages();
Assert.That(variant1Called, Is.EqualTo(1));