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.

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

Rare NRE on Telepathy.Server.GetClientAddress() #123

Open
cdanek opened this issue Nov 22, 2022 · 2 comments
Open

Rare NRE on Telepathy.Server.GetClientAddress() #123

cdanek opened this issue Nov 22, 2022 · 2 comments

Comments

@cdanek
Copy link

cdanek commented Nov 22, 2022

I apologize for being rather light on information, this issue crops up extremely rarely for us in production. It appears as if something is causing a null reference exception on connection events line 326 in Server.cs. My server is containerized and runs in Azure's cloud, but I don't have a lot of excellent infrastructure for tracking these sorts of issues. All I have is a stack trace from the event:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Telepathy.Server.GetClientAddress(Int32 connectionId) in /home/runner/work/ISG-Master/ISG-Master/Telepathy/Server.cs:line 326
   at <my code>.HandleConnectedEvent(Int32 connectionId) in <my code>/TelepathyServerSocket.cs:line 108
   at Telepathy.Server.Tick(Int32 processLimit, Func_1 checkEnabled) in /home/runner/work/ISG-Master/ISG-Master/Telepathy/Server.cs:line 378
   at <my code>.Poll() in <my code>/TelepathyServerSocket.cs:line 67
   at <my code>.RunAsync() in <my code>/NetworkManager.cs:line 86
   at Program.<Main>$(String[] args) in <my code>/Program.cs:line 48

I don't quite know what's causing the NRE, but might propose that the line is rewritten with fewer chained method calls to make it more obvious in the future? I'll submit a PR for it, but if it's obvious to the author what the cause is, great. :)

@PyrateAkananto
Copy link
Contributor

I can consistently reproduce a NullReferenceException (NRE) by calling GetClientAddress within the callback OnDisconnected of Telepathy.Server. In the line return ((IPEndPoint)connection.client.Client.RemoteEndPoint).Address.ToString(); the second Client (with capital C) is null in this case. I assume when the callback OnDisconnected is called the connection is still within readonly ConcurrentDictionary<int, ConnectionState> clients but public Socket Client { get; set; } of System.Net.Sockets.TcpClient is already null.

In your pull request #124 I would not only split the call chain but even add if (... != null) checks. If one of those actually is null, I would simply return an empty string.

@cdanek
Copy link
Author

cdanek commented Jan 8, 2023

Thanks for that. I've updated my code locally and added some NRE checks to the PR. They might not all be necessary, but they're inexpensive checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants