Unity-Technologies / com.unity.netcode.gameobjects

Netcode for GameObjects is a high-level netcode SDK that provides networking capabilities to GameObject/MonoBehaviour workflows within Unity and sits on top of underlying transport layer.
MIT License
2.15k stars 434 forks source link

Server shutdown when receiving a disconnection message from an already disconnected client #2858

Open Goreduc opened 7 months ago

Goreduc commented 7 months ago

Description

When connection manager is receiving disconnection message, it uses TransportIdToClientIdMap and ClientIdToTransportIdMap to find the client and manage the disconnection. However if the message is received twice, the client no more exists the second time in the maps. A warning is raised

[Netcode] Trying to get the NGO client ID map for the transport ID (4178184557) but did not find the map entry! Returning default transport ID value

But the default value "0" correspond to the server. As a consequence, the disconnection process is executed for the server

Reproduce Steps

The bug is easily reproductible using Facepunch, but should be reproductible in other cases

Actual Outcome

Server is disconnected

Expected Outcome

A message adressed to a disconnected client should be ignored

Screenshots

Environment

Additional Context

Discussion can be found here : https://forum.unity.com/threads/ngo-facepunch-problem-when-approval-is-refused-for-a-client.1525018

NoelStephensUnity commented 6 months ago

@Goreduc I will look into this issue, but I would like to point out that this is a bug in the facepunch NetworkTransport implementation that then results in unexpected erroneous behavior (i.e. getting the disconnect message twice from the same client).

@fluong6 I migrated this down to medium since it is an issue with FacePunch and we are trying to prioritize other high priority issues that are due to our supported scope. Keep the imported issue as we should try to make this part of our code more fault tolerant and I do believe the fix should not be that time consuming.