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

Jump to bottom

fix: network objects not destroyed on server stop #468

Merged
merged 4 commits into from
Dec 19, 2020
Merged

fix: network objects not destroyed on server stop #468

merged 4 commits into from
Dec 19, 2020

Conversation

uweeby
Copy link
Member

@uweeby uweeby commented Nov 3, 2020

failing test for #467

@uweeby
Copy link
Member Author

uweeby commented Nov 3, 2020

I dont currently understand why these tests are failing after this change was made. Will have to dig more or find a different solution.

@phodoval
Copy link
Contributor

@uweenukr I started digging into this and it seems none of the tests actually fail themselves, it's that TearDown starts to give this error for the failing tests:

TearDown : Unhandled log message: '[Error] : Closed connection: connection(127.0.0.1:0). Invalid message System.NullReferenceException: Object reference not set to an instance of an object
  at Mirror.NetworkIdentity.AddObserver (Mirror.INetworkConnection conn) [0x00066] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\NetworkIdentity.cs:980 
  at Mirror.ServerObjectManager.SpawnObserversForConnection (Mirror.INetworkConnection conn) [0x000ed] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\ServerObjectManager.cs:176 
  at Mirror.ServerObjectManager.SetClientReady (Mirror.INetworkConnection conn) [0x00036] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\ServerObjectManager.cs:674 
  at Mirror.ServerObjectManager.OnClientReadyMessage (Mirror.INetworkConnection conn, Mirror.ReadyMessage msg) [0x00021] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\ServerObjectManager.cs:714 
  at Mirror.NetworkConnection+<>c__DisplayClass22_0`1[T].<MessageHandler>g__AdapterFunction|0 (Mirror.INetworkConnection conn, Mirror.NetworkReader reader, System.Int32 channelId) [0x0001f] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\NetworkConnection.cs:121 
  at Mirror.NetworkConnection.InvokeHandler (System.Int32 msgType, Mirror.NetworkReader reader, System.Int32 channelId) [0x00010] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\NetworkConnection.cs:259 
  at Mirror.NetworkConnection.TransportReceive (System.ArraySegment`1[T] buffer, System.Int32 channelId) [0x0000e] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\NetworkConnection.cs:295 '. Use UnityEngine.TestTools.LogAssert.Expect
---
: Closed connection: connection(127.0.0.1:0). Invalid message System.NullReferenceException: Object reference not set to an instance of an object
  at Mirror.NetworkIdentity.AddObserver (Mirror.INetworkConnection conn) [0x00066] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\NetworkIdentity.cs:980 
  at Mirror.ServerObjectManager.SpawnObserversForConnection (Mirror.INetworkConnection conn) [0x000ed] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\ServerObjectManager.cs:176 
  at Mirror.ServerObjectManager.SetClientReady (Mirror.INetworkConnection conn) [0x00036] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\ServerObjectManager.cs:674 
  at Mirror.ServerObjectManager.OnClientReadyMessage (Mirror.INetworkConnection conn, Mirror.ReadyMessage msg) [0x00021] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\ServerObjectManager.cs:714 
  at Mirror.NetworkConnection+<>c__DisplayClass22_0`1[T].<MessageHandler>g__AdapterFunction|0 (Mirror.INetworkConnection conn, Mirror.NetworkReader reader, System.Int32 channelId) [0x0001f] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\NetworkConnection.cs:121 
  at Mirror.NetworkConnection.InvokeHandler (System.Int32 msgType, Mirror.NetworkReader reader, System.Int32 channelId) [0x00010] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\NetworkConnection.cs:259 
  at Mirror.NetworkConnection.TransportReceive (System.ArraySegment`1[T] buffer, System.Int32 channelId) [0x0000e] in C:\Dev\games\MirrorNG\Assets\Mirror\Runtime\NetworkConnection.cs:295

It seems like the mock connection still receives some data from mock transport after the StopHost() in TearDown, which leads to NetIdentity trying to add new observer when SOM is already destroyed, thus causing the NRE. So it seems your fix actually revealed some hidden timing issue or something.

@paulpach paulpach changed the title bug: network objects not destroyed on server stop fix: network objects not destroyed on server stop Nov 23, 2020
@sonarcloud
Copy link

sonarcloud bot commented Dec 19, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@uweeby uweeby merged commit abf5f2f into master Dec 19, 2020
@uweeby uweeby deleted the bugNoDestroyOnSTop branch December 19, 2020 17:16
github-actions bot pushed a commit that referenced this pull request Dec 19, 2020
@semantic-release-bot
chore(release): 60.0.1 [skip ci]
f15a173 
## [60.0.1](v60.0.0...v60.0.1) (2020-12-19)

### Bug Fixes

* network objects not destroyed on server stop ([#468](#468)) ([abf5f2f](abf5f2f))
@github-actions
Copy link
Contributor

🎉 This PR is included in version 60.0.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@phodoval phodoval mentioned this pull request Dec 19, 2020
Closed
@uweeby uweeby mentioned this pull request Jan 4, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@uweeby @phodoval