search

espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.62k stars 7.41k forks source link

WifiClientSecure runs out of memory after multiple MQTT publish operations when WiFiClientSecure::loadCACert is used #9636

Closed the-butcher closed 5 months ago

the-butcher commented 5 months ago

Board

featheresp32-s2

Device Description

Adafruit ESP32-S2 Feather with BME280 Sensor https://www.adafruit.com/product/5303

platformio.ini as follows:

[env:featheresp32-s2] platform = espressif32 board = featheresp32-s2 framework = arduino board_build.f_cpu = 80000000L board_build.f_flash = 80000000L board_build.flash_mode = qio board_build.partitions = min_spiffs.csv monitor_speed = 115200 lib_deps = adafruit/Adafruit NeoPixel@^1.12.0 adafruit/Adafruit BusIO@^1.15.0 SPI@^2.0.0 adafruit/SdFat - Adafruit Fork@^2.2.3 sensirion/Sensirion I2C SCD4x@^0.4.0 adafruit/Adafruit EPD@^4.5.4 adafruit/Adafruit BME280 Library@^2.2.4 adafruit/Adafruit LC709203F@^1.3.4 adafruit/RTClib@^2.1.3 bblanchon/ArduinoJson@5.13.4 ottowinter/ESPAsyncWebServer-esphome@^3.1.0 ricmoo/QRCode@^0.0.1 knolleary/PubSubClient@^2.8 build_flags = -DCORE_DEBUG_LEVEL=0

Hardware Configuration

Yes

Version

other

IDE Name

PlatformIO

Operating System

Windows 10

Flash frequency

80mHz

PSRAM enabled

no

Upload speed

not specified

Description

A CO2 measuring device tries to publish it's measurements through MQTT in certain intervals, i.e. 3 minutes. The device deep-sleeps between measurements, therefore WifiClient and PubSubClient need to be instantiated and destroyed every time new measurements need to be published.

Since the user can upload the mqtt broker certificate it needs to be kept in a file on SD-card. Each tim a new instance of WifiClientSecure is created, the certificate file is opened and passed to that client through: WiFiClientSecure::loadCACert(Stream& stream, size_t size)

ESP.getFreeHeap() returns ~70000bytes when first measurements are published, but reduces with each publish operation, until at ~60.000bytes the following error shows:

[start_ssl_client():273]: (-32512) SSL - Memory allocation failed

With SSL not correctly set up any more, further connections fail and subsequently mqtt publish fails too.

Workaround: free heap roughly reduces by the size of the certificate file each time. Looking at the code of WifiClientSecure it seems that a certificate, once loaded, never is freed when the client instance is destroyed.

When loading the certificate manually, then using. WiFiClientSecure::setCACert (const char *rootCA) rather than WiFiClientSecure::loadCACert(Stream& stream, size_t size) and freeing the loaded data before destroying the client, the memory leak appears to be fixed.

Sketch

//--------------------------------------------------------------------------------
// first solution (memory leak)

// instantiate wifi-client
WiFiClient* wifiClient;
char* certFileData = NULL;
if (crt != "" && ModuleCard::existsPath(crt)) {
    wifiClient = new WiFiClientSecure();
    File32 certFile;
    certFile.open(crt, O_RDONLY);
    ((WiFiClientSecure*)wifiClient)->loadCACert(certFile, certFile.size());
    certFile.close();
} else {
    wifiClient = new WiFiClient();
}

// instantiate mqtt-client
PubSubClient* mqttClient;
mqttClient = new PubSubClient(*wifiClient);
mqttClient->setServer(srv, prt);
if (usr != "" && pwd != "") {
    mqttClient->connect(cli, usr, pwd, 0, 0, 0, 0, 0);
} else {
    mqttClient->connect(cli);
}

if (mqttClient->connected()) {

    // mqtt publishing happening here, around 100bytes/measurement, 300bytes total

    mqttClient->disconnect();  // calls stop() on wificlient

}

// redundant call to wifi-client stop
wifiClient->stop();

delete mqttClient; // releases some buffer
delete wifiClient; // calls stop (again) and deletes an internal sslclient instance
mqttClient = NULL;
wifiClient = NULL;

//--------------------------------------------------------------------------------
// alternative solution (no memory leak)

// instantiate wifi-client
WiFiClient* wifiClient;
char* certFileData = NULL;
if (mqtt.crt != "" && ModuleCard::existsPath(mqtt.crt)) {
    wifiClient = new WiFiClientSecure();
    File32 certFile;
    certFile.open(mqtt.crt, O_RDONLY);
    certFileData = (char*)malloc(certFile.size() + 1);
    certFile.readBytes(certFileData, certFile.size());
    ((WiFiClientSecure*)wifiClient)->setCACert(certFileData);
    certFile.close();
} else {
    wifiClient = new WiFiClient();
}

// instantiate mqtt-client
PubSubClient* mqttClient;
mqttClient = new PubSubClient(*wifiClient);
mqttClient->setServer(mqtt.srv, mqtt.prt);
if (mqtt.usr != "" && mqtt.pwd != "") {
    mqttClient->connect(mqtt.cli, mqtt.usr, mqtt.pwd, 0, 0, 0, 0, 0);
} else {
    mqttClient->connect(mqtt.cli);  // connect without credentials
}

if (mqttClient->connected()) {

    // mqtt publishing happening here, around 100bytes/measurement, 300bytes total

    mqttClient->disconnect();  // calls stop() on wificlient

}

// redundant call to wifi-client stop
wifiClient->stop(); 
if (certFileData != NULL) {
    free(const_cast<char*>(certFileData));  // free the memory used to hold the certificate
    certFileData = NULL;
}

delete mqttClient;  // releases some buffer
delete wifiClient;  // calls stop (again) and deletes an internal sslclient instance
mqttClient = NULL;
wifiClient = NULL;

Debug Message

[start_ssl_client():273]: (-32512) SSL - Memory allocation failed

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

VojtechBartoska commented 5 months ago

Hello Hannes, what version of Arduino Core are you running? I do not see it in your issue report.

the-butcher commented 5 months ago

Sorry for not including in the first place. Is the information below sufficient?

PACKAGES:

JAndrassy commented 5 months ago

@VojtechBartoska the 3.0.0 has the problem too.

EDIT: or doesn't. but has a problem related to the fix of that problem. If setCaCert is first used with the cert on stack or global, and the load is invoked, it will attempt to delete the instance on stack or global

VojtechBartoska commented 5 months ago

@me-no-dev can you please take a look on this? Thanks!

VojtechBartoska commented 5 months ago

please when you have some time, take a look on this ticker and help with triage @me-no-dev. Thanks a lot!

me-no-dev commented 5 months ago

I see the problem