eclipse-paho / eclipse-paho/paho.mqtt.java

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

Open
#1,093 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.