Race condition between disconnect() and close() leaves zombie client/threads behind

[MortronMeymo]

Please fill out the form below before submitting, thank you!

• Bug exists Release Version 1.2.5 ( Master Branch)
• Bug exists in MQTTv3 Client on Snapshot Version 1.2.6-SNAPSHOT (Develop Branch)
• Bug exists in MQTTv5 Client on Snapshot Version 1.2.6-SNAPSHOT (Develop Branch)

If this is a bug regarding the Android Service, please raise the bug here instead: https://github.com/eclipse/paho.mqtt.android/issues/new

The following code:

MqttAsyncClient client = ....; // connected client
client.disconnect();
client.close();

results in the disconnect never really being completed. It does send the MQTT Disconnect package (which then causes the broker to also send a disconnect and close the connection) but then never closes the socket or stop its thread leaving always at least three zombie threads that will never be cleaned up...

The problem seems to be a race condition between disconnect() and close():

1. The disconnect() inside the ClientComms (both v3 and v5) sets the state to DISCONNECTING and starts a new thread to do the actual disconnect.
2. The close sees this DISCONNECTING state and does nothing except setting closePending to true.
3. The new thread for disconnecting (from 1) first does some internal cleanup, then sends the Disconnect message to the broker and waits for that to be written.
4. The disconnect thread calls shutdownConnection(...) which before doing anything checks among other things closePending and if that is true just returns doing nothing.

So if 2 happens before 4 the client is left in this broken state and never does the cleanup. On my machine above code manages to hit this race condition 100% of the time.

[MortronMeymo]

I'm planning to try my hand at a PR to fix this, let's see.