Memory leak in esp_mqtt_destroy_config() (IDFGH-2293)

[eespi]

• Module or chip used: ESP32-WROOM-32D
• IDF version: v4.0-beta1-279-g0a03a55
• Build System: Make
• Compiler version: 8.2.0
• Operating System: Linux
• Power Supply: external 3.3V

Problem Description

Calling esp_mqtt_destroy_client leaks about 216 bytes of memory because even loop is not freed.

The leak is located inside esp_mqtt_destroy_config which should call esp_event_loop_delete but a is memset clears the handle before it is freed.

After some time of internet outage, the application will stuck becuse no memory available and error mbedtls ssl setup returned -0x7f00

Expected Behavior

esp_event_loop_delete should be called before memset, or vice-versa

Actual Behavior

Steps to repropduce

1. call esp_mqtt_client_init
2. call esp_mqtt_client_destroy
3. loop

Code to reproduce this issue

In file mqtt_client.c

static esp_err_t esp_mqtt_destroy_config(esp_mqtt_client_handle_t client)
{
    mqtt_config_storage_t *cfg = client->config;
    free(cfg->host);
    free(cfg->uri);
    free(cfg->path);
    free(cfg->scheme);
    memset(cfg, 0, sizeof(mqtt_config_storage_t));  
    !!!!!!!!!!!!!!!!!!!! NOTE client->config->event_loop_handle now is NULL !!!!!!!!!!!!!!!!!!!! 
    free(client->connect_info.will_topic);
    free(client->connect_info.will_message);
    free(client->connect_info.client_id);
    free(client->connect_info.username);
    free(client->connect_info.password);
    memset(&client->connect_info, 0, sizeof(mqtt_connect_info_t));
#ifdef MQTT_SUPPORTED_FEATURE_EVENT_LOOP
    !!!!!!!!!!!!!!!!!!!!  NOTE This never gets called !!!!!!!!!!!!!!!!!!!! 
    if (client->config->event_loop_handle) { 
        esp_event_loop_delete(client->config->event_loop_handle);
    }
#endif
    free(client->config);
    return ESP_OK;
}

This fixes the problem, allowing esp_event_loop_delete to be called:

static esp_err_t esp_mqtt_destroy_config(esp_mqtt_client_handle_t client)
{
    mqtt_config_storage_t *cfg = client->config;
    free(cfg->host);
    free(cfg->uri);
    free(cfg->path);
    free(cfg->scheme);
    free(client->connect_info.will_topic);
    free(client->connect_info.will_message);
    free(client->connect_info.client_id);
    free(client->connect_info.username);
    free(client->connect_info.password);
    memset(&client->connect_info, 0, sizeof(mqtt_connect_info_t));
#ifdef MQTT_SUPPORTED_FEATURE_EVENT_LOOP
    if (client->config->event_loop_handle) {
        esp_event_loop_delete(client->config->event_loop_handle);
    }
#endif
    memset(cfg, 0, sizeof(mqtt_config_storage_t));
    free(client->config);
    return ESP_OK;
}

Debug Logs

This is the log of heap trace recordered from init to destroy

3 allocations trace (100 entry buffer)
24 bytes (@ 0x3ffd8d30) allocated CPU 0 ccount 0xd7ac8400 caller 0x4018380e:0x4014c5db
0x4018380e: esp_event_loop_create at /media/paolo/Data/working/FW/esp32/esp-idf/components/esp_event/esp_event.c:418

0x4014c5db: esp_mqtt_client_init at /media/paolo/Data/working/FW/esp32/esp-idf/components/mqtt/esp-mqtt/mqtt_client.c:391

104 bytes (@ 0x3ffd8d4c) allocated CPU 0 ccount 0xd7ac8cb0 caller 0x4008e63a:0x4018383e
0x4008e63a: xQueueGenericCreate at /media/paolo/Data/working/FW/esp32/esp-idf/components/freertos/queue.c:392 (discriminator 2)

0x4018383e: esp_event_loop_create at /media/paolo/Data/working/FW/esp32/esp-idf/components/esp_event/esp_event.c:424

88 bytes (@ 0x3ffd8db8) allocated CPU 0 ccount 0xd7ac94f4 caller 0x4008e63a:0x4008e81b
0x4008e63a: xQueueGenericCreate at /media/paolo/Data/working/FW/esp32/esp-idf/components/freertos/queue.c:392 (discriminator 2)

0x4008e81b: xQueueCreateMutex at /media/paolo/Data/working/FW/esp32/esp-idf/components/freertos/queue.c:499

216 bytes 'leaked' in trace (3 allocations)
total allocations 27 total frees 24

This is the log of heap trace recordered after the fix:

0 allocations trace (100 entry buffer)
0 bytes 'leaked' in trace (0 allocations)
total allocations 27 total frees 27
Free Heap Size 2: 168372

[ESP-Marius]

Yeah, this looks like an oversight. We will fix this and close the issue when it has been deployed to Github.

Thank you for reporting this.

[AxelLin]

Hi @david-cermak
The fix has been in esp-mqtt tree for a month but not yet in esp-idf.
I'm wondering if it is normal taking such long time to merge the fix into esp-idf
especially when the fix is so trivial.

[ESP-Marius]

@AxelLin Hi,
Yeah, sorry for being slow about getting it into esp-idf, I'll push a merge request for it today.

Sorry for keeping you waiting.