search

espressif / esp-mqtt

ESP32 mqtt component
Apache License 2.0
603 stars 255 forks source link

PING/PUBLISH clash #90

Closed neilyoung closed 5 years ago

neilyoung commented 5 years ago

I'm monitoring a strange problem which I think to have identified in the way a PING is sent. It seems to me, that the observation provided here https://www.esp32.com/viewtopic.php?t=3435&start=10 is true: Since PING and PUBLISH are running on different threads they can destroy/disturb each other. The symptom on the receiving end is an unusually high loss of connectivity, in parallel with frequent re-establishments of connectivity. If I comment the entire PING logic in mqtt_client.c, everything works.

The problem manifests especially under weak network conditions. The worst result seen where incomplete PUBLISH packages (e.g. those have been sent with say 8kB and received CONSTANTLY and REPRODUCIBLE with 55 bytes) and have been delivered as valid PUBLISH messages on client side.

I haven't investigated in details, but the 55 byte message, MOST the time incorrectly length coded (as 64 byte payload coded as two byte length 0xC0 0x00 instead of single byte 0x40), is a repeating pattern. I just got a clue about what happens monitoring the output of mqtt_client.c, which always lost and re-established connectivity and all the time reporting a SEND issue with the PING simultaneously.

Using the Xcode "Network conditioner", which allowed to emulate worse network conditions I was able to provoke that situation relatively quickly and detect the problem.

I would suggest to remove the PING from the separate thread and hand out the control to the user which issues the PUBLISH.

david-cermak commented 5 years ago

Hi @neilyoung

The library is not thread safe, so running publish from a different thread is not supported.

We will provide a thread-safe version soon.

david-cermak commented 5 years ago

Resolved via https://github.com/espressif/esp-mqtt/commit/752953dc3be007cca4255b66a35d3087e61f6a54