Reconnect handling

[ghost]

From gbar...@gmail.com on July 29, 2010 13:59:12

A disconnect from the server is easily recognizable through Listener / disconnected. Yet there is no easy way to reconnect, which brings me to the following problems:

1. It's not possible to get the current parameters client.getRemoteAddressTCP() respective client.getRemoteAddressTCP() - both return null when the connection is dropped
2. There is no client.getTimeout() method, although the parameter is required for connect()
3. A client.reconnect() would be really great

I'm using the latest 1.01 version from SVN.

Original issue: http://code.google.com/p/kryonet/issues/detail?id=4

[ghost]

From nathan.s...@gmail.com on August 09, 2010 16:58:15

First problem: A connection is closed when the id == -1. Listener#disconnected is called before id is set to -1, so you know what connection was disconnected. This can cause the disconnected listener to fire multiple times for the same disconnect.

The fix: The id == -1 no longer indicates a closed connection. Use Connection#isConnected instead. The id will be -1 if never connected, otherwise it is the last assigned ID. This change has the potential to break existing users, but I feel it is a better approach.

Next, I added storage of the connect parameters in Client and added reconnect and reconnect(int) method. I added a ReconnectTest class to demonstrate.

Note that you cannot call connect or reconnect on the network update thread because this would block incoming messages that need to be processed during connect. This may be a bit annoying, but connect's blocking behavior makes it easy to handle failure.

Status: Fixed

[ghost]

From gbar...@gmail.com on August 10, 2010 15:39:59

Thank you very much!

I had a look at the new test case, but it doesn't use the reconnect method, instead it relies on connect(). I therefore attempted to do it on my own:

public class MyClientListener extends Listener { // .. @-Override public void disconnected(Connection connection) { try { if (!connection.isConnected()) this.client.reconnect(); } catch (IOException e) { System.out.println("Reconnect failed!"); e.printStackTrace(); } } }

Which gave me an:

Exception in thread "Client" java.lang.IllegalStateException: Cannot connect on the connection's update thread. at com.esotericsoftware.kryonet.Client.connect(Client.java:133) at com.esotericsoftware.kryonet.Client.reconnect(Client.java:203) at com.esotericsoftware.kryonet.Client.reconnect(Client.java:193) at org.gbits.t2u.ClientListener.disconnected(ClientListener.java:25) at com.esotericsoftware.kryonet.Connection.notifyDisconnected(Connection.java:243) at com.esotericsoftware.kryonet.Connection.close(Connection.java:147) at com.esotericsoftware.kryonet.Client.close(Client.java:328) at com.esotericsoftware.kryonet.Client.run(Client.java:300) at java.lang.Thread.run(Thread.java:637)

Sorry for being annoying and many thanks for the changes as well as kryonet in general ;)

[ghost]

From nathan.s...@gmail.com on August 10, 2010 15:50:57

Did you look at the right test? It uses reconnect: http://code.google.com/p/kryonet/source/browse/trunk/kryonet/test/com/esotericsoftware/kryonet/ReconnectTest.java The error you get is because, as explained above, connect() blocks until connected. You can't block the network thread to do a connect because during connection the client and server need to do some communication. If the network thread is blocked, it can't do an update to handle this communication and connect would fail.

[ghost]

From nathan.s...@gmail.com on August 10, 2010 15:52:56

Ha! You're right, the test wasn't calling reconnect. Oops. Fixed.

[ghost]

From gbar...@gmail.com on August 10, 2010 15:54:44

That's what I call a fast response ;-) Many thanks, I'll check the updated test case.

[ghost]

From gbar...@gmail.com on August 10, 2010 16:24:53

The following code does the trick (basically a copy of your test case):

public class MyClientListener extends Listener { // .. @-Override public void disconnected(Connection connection) { new Thread() { public void run() { try { System.out.println("Reconnecting: "); client.reconnect(); } catch (IOException ex) { ex.printStackTrace(); } } }.start(); } }

I'm however not sure, if that's the right way. In my understanding, the above code relies on the fact that it takes some time to start a new thread.

Wouldn't it be cleaner to i.e. add a parameter to client.connect() i.e. "reconnectCounter" with a default value of 0 (=none). One could then easily cover it in kryonet's Client.close():

public void close () { super.close(); udpRegistered = false; // Select one last time to complete closing the socket. synchronized (updateLock) { selector.wakeup(); try { selector.selectNow(); } catch (IOException ignored) { } }

if( currRetries < maxRetries)
    this.reconnect();

}

[ghost]

From nathan.s...@gmail.com on August 10, 2010 17:14:11

There is no race condition. Starting a new thread allows the network thread to update while the connect is happening.

The connect cannot happen in Client#close() because this can be called on the network thread. Also, this would prevent any handling of a reconnection failure.