eclipse-paho / eclipse-paho/paho.mqtt.java
Race condition between disconnect() and close() leaves zombie client/threads behind
- Dominant language
- Java
- Stars
- 2.3k
- Forks
- 919
- PR merge metrics
- No merged PRs in 30d
Description
Please fill out the form below before submitting, thank you!
- [x] Bug exists Release Version 1.2.5 ( Master Branch)
- [x] Bug exists in MQTTv3 Client on Snapshot Version 1.2.6-SNAPSHOT (Develop Branch)
- [x] 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.
Contributor guide
Research direction
Start by tracing disconnect() and close() in ClientComms for both MQTT v3 and v5, then follow shutdownConnection(...) and the closePending state described in the report. Reproduce the sequence with a connected MqttAsyncClient. Done means the disconnect and close race reliably performs socket and thread cleanup without leaving zombie threads.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100