lidgren / lidgren-network-gen3

Lidgren Network Library
https://groups.google.com/forum/#!forum/lidgren-network-gen3
MIT License
1.19k stars 331 forks source link

Sending a message then closing program does not flush message. #94

Open sbarisic opened 6 years ago

sbarisic commented 6 years ago

So, calling NetClient.Disconnect("Message"); and then closing the application by exiting the Main function does not seem to send the disconnect message to the server and instead the connection closes by timing out.

NetClient.FlushSendQueue() does not seem to do anything just after Disconnect either.

My workaround was

while ((Peer as NetClient)?.ServerConnection != null)
        Thread.Sleep(10);
RevoluPowered commented 6 years ago

Can you please try using this / it's what I use in my app NetClientInstance.Shutdown("Client application exited");

If that properly disconnects the client then I might be able to fix it relatively quickly.

sbarisic commented 6 years ago

Shutdown was tested by my friend, it also didn't work. That's how i was alerted to the problem. It behaves the same as Disconnect

RevoluPowered commented 6 years ago

Can you please tell me what platforms it has been tested on? I'm not able to reproduce this issue, with the same code. Testing on Windows 10.

If you can provide a code example of how you're initialising the network server I might be able to work out what's going wrong.

Also any libraries which your using would be helpful to know too.

I need to simulate the exact conditions which you're putting this through in order to find the bug.

sbarisic commented 6 years ago

ckmcgk

Lidgren_Test.zip

Here's the result and the whole project. Tested with start NetTest.exe --server && start cmd /C "NetTest.exe && pause"

RevoluPowered commented 6 years ago

Thank you for sending that, I will take a look into it later this evening. I'll update you on this tomorrow evening.

RevoluPowered commented 6 years ago

Found out the source of the issue, so what's happening is, NetServer's socket is getting cleaned up properly the first time it closes down / kind-of, then the NetClient properly disconnects.

If that same client reconnects to the server, even a new instance of it, the socket doesn't close. When the server closes for the second time, the client has to timeout.

I think the issue is in the NetClient, and I'll see if i can fix it for tomorrrow.

note it also doesn't seem to be happening all of the time.

RevoluPowered commented 6 years ago

Started writing up a unit test for this to find out what's causing it.

RevoluPowered commented 6 years ago

WIP fix for this issue, I'd wait until I've written a full test for your scenario as I just fixed another potentially related issue with socket shutdown.

https://github.com/RevoluPowered/lidgren-network-gen3/tree/feature/socket-overhaul

I've still to do the following to get this PR ready:

You can try the update in my branch at your own peril, I'd wait a day or so.

RevoluPowered commented 6 years ago

Just finished writing up a test, noticed that the network thread isn't being shutdown properly

RevoluPowered commented 6 years ago

@cartman300 can you please try my branch now, I just found the problem.

In your NetClient and NetServer when you're shutting down also add server.GetNetworkThread().Join() client.GetNetworkThread().Join()

These are new and I'll be removing them soon it's just a tempoary fix.

I'm going to make this automatic in future, like it's intended to be right now, but it will take some time.

The issue was that the network 'start' was being re-called internally when it shouldn't be, I've added a flag to prevent this from happening.

Also, the threads are no longer background threads as it doesn't really need to be in the background.

RevoluPowered commented 6 years ago

https://github.com/RevoluPowered/lidgren-network-gen3/tree/feature/socket-overhaul

sbarisic commented 6 years ago

Amazing, thank you for fixing that. I am currently working on some OpenGL 4 rendering so i don't have time to test networking. I'll make sure to report back if i find any funky bugs regarding to this.

RevoluPowered commented 6 years ago

No problem at all, thanks for the bug report it was good to know that this was broken.

RevoluPowered commented 6 years ago

Unfortunately there is a regression in the changes I made, so the fix for this will be delayed by a while. Will get back to you when I am sure they are ready for production.