realXtend / tundra

realXtend Tundra SDK, a 3D virtual world application platform.
www.realxtend.org
Apache License 2.0
84 stars 70 forks source link

Client does not cleanly disconnect from server on framework->Exit() #665

Open jonnenauha opened 11 years ago

jonnenauha commented 11 years ago

Problem If you have a ongoing connection to a server with a Tundra client instance it will not cleanly disconnect upon framework->Exit();. The server will think the client is still connected for a while, until several kNet ping messages are not getting a response. Then the client is dropped by the server. However this take about 10-20 second (I haven't timed it).

These "unclean" exits can be caused by at least three ways

Solution proposal We have a nice system in combinating the Application::ExitRequested signal, connecting it to a slot and calling Framework::CancelExit() to not let the app go down.

My suggestion is that TundraProtocolModule::Client should delay the exit if it is connected to a server. It will at the same time request a clean disconnect and when the disconnect is completed, it will call Framework::Exist() to complete the shutdown.

@cadaver Any problems in having this kind of logic in core? Is the current shutdown behavior by design? If we don't want this in core I can ofc implement it in Rocket. We have been noticing that if clients close the app window, start it up again and connect to the same server, the old dead client with his username etc. is still in the world. This does not cause major problems for us but I think we can do better with the proposal above.

Clarification: I can implement this if we want to do it.

cadaver commented 11 years ago

This is good to include in core.

Note, however, that CancelExit() is somewhat convoluted and should be avoided if possible. The disconnect in TundraProtocolModule exit should wait by default 500ms for the server to disconnect cleanly; it needs investigation why it doesn't work.

juj commented 11 years ago

The connection timeout period in kNet is set to a very conservative period by default (15 seconds). It'd be a good feature to make this a configurable parameter at kNet server startup time, and be able to set a stricter timeout at Tundra server startup.

It's always possible to make a client forcibly quit by killing Tundra process (or if Tundra crashes). If that's done, benign disconnect is not performed since the client won't send a TCP close, or a UDP disconnect packet, and kNet (just like any other networking library) must resort to the ping timeout logic to detect disconnection.

Stinkfist0 commented 11 years ago

I think we need to modify kNet in order to be able to specify the timeout ourselves.

Edit: yup: MessageConnection.cpp:

    /// The time interval after which, if we don't get a response to a PingRequest message, the connection is declared lost.
    ///\todo Make this user-defineable.
    const float connectionLostTimeout = 15.f * 1000.f;
juj commented 11 years ago

Yes, it will require hoisting the constant as a user-defineable parameter.

jonnenauha commented 11 years ago

Yeah killing the process is always there and thats fine.

I still need to look more into if framework->Exit() actually disconnects correctly. As @cadaver pointed in IRC KristalliProtocolModule dtor has a disconnect that should block for max 500 msecs. I'd guess for most cases that should be enough to disconnect properly even if the Tundra window is closed etc. But as said I'll need to run more tests if this actually happens.

If we could define during runtime the time out to lower than 15 seconds, that would be great. If it cannot be done after the kNet creation then we could make it a start parameter so we can tweak it in our servers more easily, or we can do it from code in our server module, either way would be super :)

Btw. @juj has anything significant happened in kNet? We are at least waiting for @Stinkfist0 cmake boost changes (afaik it was that) to get into stable. I think we are using stable but you have committed some stuff to other branches there. Any idea when stable will be updated?

juj commented 11 years ago

kNet master has now been merged to stable. No functional changes, only cleanup and /w4 diagnostics and some logging-related bugfixes.