[mqtt_cxx] Race condition in idf::mqtt::Client constructor (IDFGH-13475)

[snake-4]

Answers checklist.

• I have read the documentation for esp-protocols components and the issue is not addressed there.
• I have updated my esp-protocols branch (master or release) to the latest version and checked that the issue is present there.
• I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

There is a race condition in the constructor of the Client class, where the event handler may be invoked before subclass of Client is done constructing. This results in the program terminating if the dispatched event is MQTT_EVENT_CONNECTED or MQTT_EVENT_DATA since on_data and on_connected are pure virtual functions.

Pure virtual functions cannot be called before the object is fully constructed, otherwise a compiler generated termination function will be called.

The exact line is below:

esp-protocols/components/esp_mqtt_cxx/esp_mqtt_cxx.cpp

Line 166 in e425a3c

CHECK_THROW_SPECIFIC(esp_mqtt_client_start(handler.get()), mqtt::MQTTException);

This cannot be solved by making the aforementioned functions normal virtual functions, as that would result in an undelivered event instead. A lock in the constructor would also not work as the base class can't know when the subclass is done constructing. Preferably the MQTT client should have a separate function for starting the underlying client and this should not be done in the constructor.

We discovered this while integration testing our code on the Linux target with a MQTT broker on the localhost. The connection is made so fast that the subclass doesn't have time to finish constructing before an MQTT_EVENT_CONNECTED event is dispatched.