ibm-watson-iot / iot-java

Client libraries and samples for connecting to IBM Watson IoT using Java
https://internetofthings.ibmcloud.com
Eclipse Public License 1.0
54 stars 78 forks source link

Remove reconnect logic in lostConnection callback, use Paho's automatic reconnect function #142

Closed miketran78727 closed 5 years ago

miketran78727 commented 6 years ago

The reconnect logic is not working. If we do not use Paho Client's automatic reconnect function, we will need to create a thread to try reconnect for the users.

Here is junit test trace which shows connect() was invoked in callback thread.

Connect already in progress (32110)     at org.eclipse.paho.client.mqttv3.MqttAsyncClient.connect(MqttAsyncClient.java:734)
        at org.eclipse.paho.client.mqttv3.MqttAsyncClient.connect(MqttAsyncClient.java:716)
        at com.ibm.iotf.client.AbstractClient.connect(AbstractClient.java:244)
        at com.ibm.iotf.client.AbstractClient.connect(AbstractClient.java:294)
        at com.ibm.iotf.client.app.ApplicationClient.connect(ApplicationClient.java:179)
        at com.ibm.iotf.client.app.ApplicationClient.connectionLost(ApplicationClient.java:1140)
        at org.eclipse.paho.client.mqttv3.internal.CommsCallback.connectionLost(CommsCallback.java:292)
        at org.eclipse.paho.client.mqttv3.internal.ClientComms.shutdownConnection(ClientComms.java:423)
        at org.eclipse.paho.client.mqttv3.internal.CommsReceiver.run(CommsReceiver.java:181)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:522)
        at java.util.concurrent.FutureTask.run(FutureTask.java:277)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:191)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1160)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.lang.Thread.run(Thread.java:795)
        at org.eclipse.paho.client.mqttv3.internal.CommsReceiver.run(CommsReceiver.java:181)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:522)
        at java.util.concurrent.FutureTask.run(FutureTask.java:277)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:191)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1160)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.lang.Thread.run(Thread.java:795)

        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1160)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.lang.Thread.run(Thread.java:795)

        at org.eclipse.paho.client.mqttv3.MqttAsyncClient.connect(MqttAsyncClient.java:734)
        at org.eclipse.paho.client.mqttv3.MqttAsyncClient.connect(MqttAsyncClient.java:716)
        at com.ibm.iotf.client.AbstractClient.connect(AbstractClient.java:244)
        at com.ibm.iotf.client.AbstractClient.connect(AbstractClient.java:294)
        at com.ibm.iotf.client.app.ApplicationClient.connect(ApplicationClient.java:179)
        at com.ibm.iotf.client.app.ApplicationClient.connectionLost(ApplicationClient.java:1140)
        at org.eclipse.paho.client.mqttv3.internal.CommsCallback.connectionLost(CommsCallback.java:292)
        at org.eclipse.paho.client.mqttv3.internal.ClientComms.shutdownConnection(ClientComms.java:423)
        at org.eclipse.paho.client.mqttv3.internal.CommsReceiver.run(CommsReceiver.java:181)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:522)
        at java.util.concurrent.FutureTask.run(FutureTask.java:277)
icraggs commented 6 years ago

Mike - I don't think there is any reason to use anything other than Paho auto reconnect. That's what it is there for. We don't need to duplicate it. This library should allow no other setting IMO.

(You don't need a reconnect thread because calling connect in connectionLost is specifically allowed. I think what is happening here is auto reconnect and the old style operating at the same time).

miketran78727 commented 6 years ago

Ian, I agree. We need to remove this reconnect logic.

miketran78727 commented 6 years ago

By the way, Ian, autoreconect flag is off for this test. So, this could be a bug in Paho where this Connect already in progress is set. I will investigate further.

miketran78727 commented 5 years ago

Fix in master branch.