Reconnect handling

[GoogleCodeExporter]

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 reported on code.google.com by gbar...@gmail.com on 29 Jul 2010 at 11:59

[GoogleCodeExporter]

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.

Original comment by nathan.s...@gmail.com on 9 Aug 2010 at 11:58

• Changed state: Fixed

[GoogleCodeExporter]

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 ;)

Original comment by gbar...@gmail.com on 10 Aug 2010 at 10:39

[GoogleCodeExporter]

Did you look at the right test? It uses reconnect:
http://code.google.com/p/kryonet/source/browse/trunk/kryonet/test/com/esotericso
ftware/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.

Original comment by nathan.s...@gmail.com on 10 Aug 2010 at 10:50

[GoogleCodeExporter]

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

Original comment by nathan.s...@gmail.com on 10 Aug 2010 at 10:52

[GoogleCodeExporter]

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

Original comment by gbar...@gmail.com on 10 Aug 2010 at 10:54

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

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();
}

Original comment by gbar...@gmail.com on 10 Aug 2010 at 11:24

[GoogleCodeExporter]

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.

Original comment by nathan.s...@gmail.com on 11 Aug 2010 at 12:14