Skip to content

fix: Fixed a few memory leak cases when shutting down NetworkManager … #1466

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
3ad516f
fix: Fixed a few memory leak cases when shutting down NetworkManager …
ShadauxCat Nov 12, 2021
46c12b1
Merge branch 'release/1.0.0' into backport/1.0.0-fix-fix_memory_leak_…
andrews-unity Nov 24, 2021
2c628d2
Merge branch 'release/1.0.0' into backport/1.0.0-fix-fix_memory_leak_…
andrews-unity Nov 24, 2021
e077c03
Merge release/1.0.0 into backport/1.0.0-fix-fix_memory_leak_on_shutdown
netcode-ci-service Nov 24, 2021
8e67bb1
Merge release/1.0.0 into backport/1.0.0-fix-fix_memory_leak_on_shutdown
netcode-ci-service Nov 24, 2021
c36c5f3
Merge release/1.0.0 into backport/1.0.0-fix-fix_memory_leak_on_shutdown
netcode-ci-service Nov 24, 2021
3555d74
Merge release/1.0.0 into backport/1.0.0-fix-fix_memory_leak_on_shutdown
netcode-ci-service Nov 24, 2021
17c7615
Merge release/1.0.0 into backport/1.0.0-fix-fix_memory_leak_on_shutdown
netcode-ci-service Nov 24, 2021
683fa67
Merge release/1.0.0 into backport/1.0.0-fix-fix_memory_leak_on_shutdown
netcode-ci-service Nov 24, 2021
6879af5
Merge release/1.0.0 into backport/1.0.0-fix-fix_memory_leak_on_shutdown
netcode-ci-service Nov 24, 2021
587d42b
Merge release/1.0.0 into backport/1.0.0-fix-fix_memory_leak_on_shutdown
netcode-ci-service Nov 24, 2021
955adfb
Merge release/1.0.0 into backport/1.0.0-fix-fix_memory_leak_on_shutdown
netcode-ci-service Nov 24, 2021
d87f473
Merge release/1.0.0 into backport/1.0.0-fix-fix_memory_leak_on_shutdown
netcode-ci-service Nov 24, 2021
4c86562
merge branch 'release/1.0.0' into 'backport/1.0.0-fix-fix_memory_leak…
0xFA11 Nov 24, 2021
6076c19
better merge
0xFA11 Nov 24, 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
47 changes: 41 additions & 6 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ public NetworkPrefabHandler PrefabHandler
}
}

private bool m_ShuttingDown;
private bool m_StopProcessingMessages;

private class NetworkManagerHooks : INetworkHooks
{
private NetworkManager m_NetworkManager;
Expand Down Expand Up @@ -116,7 +119,7 @@ public void OnAfterReceiveBatch(ulong senderId, int messageCount, int batchSizeI

public bool OnVerifyCanSend(ulong destinationId, Type messageType, NetworkDelivery delivery)
{
return true;
return !m_NetworkManager.m_StopProcessingMessages;
}

public bool OnVerifyCanReceive(ulong senderId, Type messageType)
Expand All @@ -134,7 +137,7 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType)
return false;
}

return true;
return !m_NetworkManager.m_StopProcessingMessages;
}
}

Expand Down Expand Up @@ -974,7 +977,7 @@ private void OnApplicationQuit()
// Note that this gets also called manually by OnSceneUnloaded and OnApplicationQuit
private void OnDestroy()
{
Shutdown();
ShutdownInternal();

UnityEngine.SceneManagement.SceneManager.sceneUnloaded -= OnSceneUnloaded;

Expand All @@ -994,13 +997,30 @@ private void DisconnectRemoteClient(ulong clientId)
/// Globally shuts down the library.
/// Disconnects clients if connected and stops server if running.
/// </summary>
public void Shutdown()
/// <param name="discardMessageQueue">
/// If false, any messages that are currently in the incoming queue will be handled,
/// and any messages in the outgoing queue will be sent, before the shutdown is processed.
/// If true, NetworkManager will shut down immediately, and any unprocessed or unsent messages
/// will be discarded.
/// </param>
public void Shutdown(bool discardMessageQueue = false)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo(nameof(Shutdown));
}

m_ShuttingDown = true;
m_StopProcessingMessages = discardMessageQueue;
}

internal void ShutdownInternal()
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfo(nameof(ShutdownInternal));
}

if (IsServer)
{
// make sure all messages are flushed before transport disconnect clients
Expand Down Expand Up @@ -1077,6 +1097,7 @@ public void Shutdown()

if (SpawnManager != null)
{
SpawnManager.CleanupAllTriggers();
SpawnManager.DestroyNonSceneObjects();
SpawnManager.ServerResetShudownStateForSceneObjects();

Expand Down Expand Up @@ -1165,6 +1186,11 @@ private void OnNetworkPreUpdate()
return;
}

if (m_ShuttingDown && m_StopProcessingMessages)
{
return;
}

// Only update RTT here, server time is updated by time sync messages
var reset = NetworkTimeSystem.Advance(Time.deltaTime);
if (reset)
Expand All @@ -1181,9 +1207,18 @@ private void OnNetworkPreUpdate()

private void OnNetworkPostLateUpdate()
{
MessagingSystem.ProcessSendQueues();
NetworkMetrics.DispatchFrame();

if (!m_ShuttingDown || !m_StopProcessingMessages)
{
MessagingSystem.ProcessSendQueues();
NetworkMetrics.DispatchFrame();
}
SpawnManager.CleanupStaleTriggers();

if (m_ShuttingDown)
{
ShutdownInternal();
}
}

/// <summary>
Expand Down
33 changes: 24 additions & 9 deletions com.unity.netcode.gameobjects/Runtime/Core/SnapshotSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -465,30 +465,45 @@ internal void ReadIndex(in SnapshotDataMessage message)
}
}

internal void SpawnObject(SnapshotSpawnCommand spawnCommand)
internal void SpawnObject(SnapshotSpawnCommand spawnCommand, ulong srcClientId)
{
if (m_NetworkManager)
{
var networkObject = m_NetworkManager.SpawnManager.CreateLocalNetworkObject(false,
spawnCommand.GlobalObjectIdHash, spawnCommand.OwnerClientId, (spawnCommand.ParentNetworkId == spawnCommand.NetworkObjectId) ? spawnCommand.NetworkObjectId : spawnCommand.ParentNetworkId, spawnCommand.ObjectPosition,
spawnCommand.ObjectRotation);
NetworkObject networkObject;
if (spawnCommand.ParentNetworkId == spawnCommand.NetworkObjectId)
{
networkObject = m_NetworkManager.SpawnManager.CreateLocalNetworkObject(false,
spawnCommand.GlobalObjectIdHash, spawnCommand.OwnerClientId, null, spawnCommand.ObjectPosition,
spawnCommand.ObjectRotation);
}
else
{
networkObject = m_NetworkManager.SpawnManager.CreateLocalNetworkObject(false,
spawnCommand.GlobalObjectIdHash, spawnCommand.OwnerClientId, spawnCommand.ParentNetworkId, spawnCommand.ObjectPosition,
spawnCommand.ObjectRotation);
}

m_NetworkManager.SpawnManager.SpawnNetworkObjectLocally(networkObject, spawnCommand.NetworkObjectId,
true, spawnCommand.IsPlayerObject, spawnCommand.OwnerClientId, false);
//todo: discuss with tools how to report shared bytes
m_NetworkManager.NetworkMetrics.TrackObjectSpawnReceived(srcClientId, networkObject, 8);
}
else
{
MockSpawnObject(spawnCommand);
}
}

internal void DespawnObject(SnapshotDespawnCommand despawnCommand)
internal void DespawnObject(SnapshotDespawnCommand despawnCommand, ulong srcClientId)
{
if (m_NetworkManager)
{
m_NetworkManager.SpawnManager.SpawnedObjects.TryGetValue(despawnCommand.NetworkObjectId,
out NetworkObject networkObject);

m_NetworkManager.SpawnManager.OnDespawnObject(networkObject, true);
//todo: discuss with tools how to report shared bytes
m_NetworkManager.NetworkMetrics.TrackObjectDestroyReceived(srcClientId, networkObject, 8);
}
else
{
Expand All @@ -497,7 +512,7 @@ internal void DespawnObject(SnapshotDespawnCommand despawnCommand)
}


internal void ReadSpawns(in SnapshotDataMessage message)
internal void ReadSpawns(in SnapshotDataMessage message, ulong srcClientId)
{
SnapshotSpawnCommand spawnCommand;
SnapshotDespawnCommand despawnCommand;
Expand All @@ -516,7 +531,7 @@ internal void ReadSpawns(in SnapshotDataMessage message)

// Debug.Log($"[Spawn] {spawnCommand.NetworkObjectId} {spawnCommand.TickWritten}");

SpawnObject(spawnCommand);
SpawnObject(spawnCommand, srcClientId);
}
for (var i = 0; i < message.Despawns.Length; i++)
{
Expand All @@ -532,7 +547,7 @@ internal void ReadSpawns(in SnapshotDataMessage message)

// Debug.Log($"[DeSpawn] {despawnCommand.NetworkObjectId} {despawnCommand.TickWritten}");

DespawnObject(despawnCommand);
DespawnObject(despawnCommand, srcClientId);
}
}

Expand Down Expand Up @@ -1077,7 +1092,7 @@ internal void HandleSnapshot(ulong clientId, in SnapshotDataMessage message)
ReadBuffer(message);
ReadIndex(message);
ReadAcks(clientId, m_ClientData[clientId], message, GetConnectionRtt(clientId));
ReadSpawns(message);
ReadSpawns(message, clientId);
}

// todo --M1--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public MessagingSystem(IMessageSender messageSender, object owner, IMessageProvi
}
}

public void Dispose()
public unsafe void Dispose()
{
if (m_Disposed)
{
Expand All @@ -113,6 +113,14 @@ public void Dispose()
{
CleanupDisconnectedClient(kvp.Key);
}

for (var queueIndex = 0; queueIndex < m_IncomingMessageQueue.Length; ++queueIndex)
{
// Avoid copies...
ref var item = ref m_IncomingMessageQueue.GetUnsafeList()->ElementAt(queueIndex);
item.Reader.Dispose();
}

m_IncomingMessageQueue.Dispose();
m_Disposed = true;
}
Expand Down Expand Up @@ -253,11 +261,15 @@ public void HandleMessage(in MessageHeader header, FastBufferReader reader, ulon

internal unsafe void ProcessIncomingMessageQueue()
{
for (var i = 0; i < m_IncomingMessageQueue.Length; ++i)
for (var index = 0; index < m_IncomingMessageQueue.Length; ++index)
{
// Avoid copies...
ref var item = ref m_IncomingMessageQueue.GetUnsafeList()->ElementAt(i);
ref var item = ref m_IncomingMessageQueue.GetUnsafeList()->ElementAt(index);
HandleMessage(item.Header, item.Reader, item.SenderId, item.Timestamp);
if (m_Disposed)
{
return;
}
}

m_IncomingMessageQueue.Clear();
Expand Down Expand Up @@ -461,16 +473,16 @@ internal unsafe void ProcessSendQueues()
try
{
m_MessageSender.Send(clientId, queueItem.NetworkDelivery, queueItem.Writer);

for (var hookIdx = 0; hookIdx < m_Hooks.Count; ++hookIdx)
{
m_Hooks[hookIdx].OnAfterSendBatch(clientId, queueItem.BatchHeader.BatchSize, queueItem.Writer.Length, queueItem.NetworkDelivery);
}
}
finally
{
queueItem.Writer.Dispose();
}

for (var hookIdx = 0; hookIdx < m_Hooks.Count; ++hookIdx)
{
m_Hooks[hookIdx].OnAfterSendBatch(clientId, queueItem.BatchHeader.BatchSize, queueItem.Writer.Length, queueItem.NetworkDelivery);
}
}
sendQueueItem.Clear();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,24 @@ internal unsafe void CleanupStaleTriggers()
m_Triggers.Remove(staleKeys[i]);
}
}
/// <summary>
/// Cleans up any trigger that's existed for more than a second.
/// These triggers were probably for situations where a request was received after a despawn rather than before a spawn.
/// </summary>
internal void CleanupAllTriggers()
{
foreach (var kvp in m_Triggers)
{
foreach (var data in kvp.Value.TriggerData)
{
data.Reader.Dispose();
}

kvp.Value.TriggerData.Dispose();
}

m_Triggers.Clear();
}

internal void RemoveOwnership(NetworkObject networkObject)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void TestStartupServerState()
public void TestFlagShutdown()
{
m_NetworkManager.StartServer();
m_NetworkManager.Shutdown();
m_NetworkManager.ShutdownInternal();

Assert.False(m_NetworkManager.IsServer);
Assert.False(m_NetworkManager.IsClient);
Expand Down
Loading