SmingHub / Sming

Sming - powerful open source framework simplifying the creation of embedded C++ applications.
https://sming.readthedocs.io
GNU Lesser General Public License v3.0
1.48k stars 348 forks source link

Memory leak in MqttClient connect #1740

Closed verybadsoldier closed 5 years ago

verybadsoldier commented 5 years ago

It seems that that trying to connect with MqttClient causes a memory leak. When the MQTT broker is not reachable and then retrying regularly then at some point you run out of heap.

Should be easily reproducible with the sample MqttClient_Hello. While I used this exact code (basically only publish removed):

#include <user_config.h>
#include <SmingCore.h>

// If you want, you can define WiFi settings globally in Eclipse Environment Variables
#ifndef WIFI_SSID
#define WIFI_SSID "<ssid>" // Put you SSID and Password here
#define WIFI_PWD "<pw>"
#endif

// For testing purposes, try a few different URL formats
#define MQTT_URL1 "mqtt://attachix.com:1883"
#define MQTT_URL2 "mqtts://attachix.com:8883" // (Need ENABLE_SSL)
#define MQTT_URL3 "mqtt://frank:fiddle@192.168.100.107:1883"

#define MQTT_URL "mqtt://192.168.5.2:555"

//const Url url("192.168.5.2:555");

// Forward declarations
void startMqttClient();
void onMessageReceived(String topic, String message);

MqttClient mqtt;

Timer procTimer;

// Check for MQTT Disconnection
void checkMQTTDisconnect(TcpClient& client, bool flag)
{
if(flag == true) {
Serial.println("MQTT Broker Disconnected!!");
} else {
Serial.println("MQTT Broker Unreachable!!");
}

// Restart connection attempt after few seconds
procTimer.initializeMs(2 * 1000, startMqttClient).start(); // every 2 seconds
}

void onMessageDelivered(uint16_t msgId, int type)
{
Serial.printf("Message with id %d and QoS %d was delivered successfully.", msgId,
 (type == MQTT_MSG_PUBREC ? 2 : 1));
}

// Publish our message
void publishMessage()
{
if(mqtt.getConnectionState() != eTCS_Connected) {
startMqttClient(); // Auto reconnect
}

Serial.print("Let's publish message now. Memory free=");
Serial.println(system_get_free_heap_size());
mqtt.publish("main/frameworks/sming", "Hello friends, from Internet of things :)");

mqtt.publishWithQoS("important/frameworks/sming", "Request Return Delivery", 1, false,
onMessageDelivered); // or publishWithQoS
}

// Callback for messages, arrived from MQTT server
void onMessageReceived(String topic, String message)
{
Serial.print(topic);
Serial.print(":\r\n\t"); // Pretify alignment for printing
Serial.println(message);
}

// Run MQTT client
void startMqttClient()
{
procTimer.stop();

Serial.print("Let's publish message now. Memory free=");
Serial.println(system_get_free_heap_size());

// 1. [Setup]
if(!mqtt.setWill("last/will", "The connection from this device is lost:(", 1, true)) {
debugf("Unable to set the last will and testament. Most probably there is not enough memory on the device.");
}

mqtt.setCompleteDelegate(checkMQTTDisconnect);
mqtt.setCallback(onMessageReceived);

#ifdef ENABLE_SSL
mqtt.addSslOptions(SSL_SERVER_VERIFY_LATER);

#include <ssl/private_key.h>
#include <ssl/cert.h>

mqtt.setSslKeyCert(default_private_key, default_private_key_len, default_certificate, default_certificate_len,
  nullptr,
  /*freeAfterHandshake*/ false);

#endif

// 2. [Connect]
Url url(MQTT_URL);
Serial.printf("Connecting to \t%s\n", url.toString().c_str());
mqtt.connect(url, "esp8266");
mqtt.subscribe("main/status/#");
}

void onConnected(IPAddress ip, IPAddress netmask, IPAddress gateway)
{
// Run MQTT client
startMqttClient();
}

void init()
{
Serial.begin(SERIAL_BAUD_RATE); // 115200 by default
Serial.systemDebugOutput(true); // Debug output to serial

WifiStation.config(WIFI_SSID, WIFI_PWD);
WifiStation.enable(true);

// Run our method when station was connected to AP (or not connected)
WifiEvents.onStationGotIP(onConnected);
}

Output is this:

--- Miniterm on /dev/ttyUSB0  115200,8,N,1 ---
--- Quit: Ctrl+] | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H ---
ota1 not set
ota2 not set
>> 0x3f␀*qURQ���␃�␛�N�{��o|�␌$␌dll ␃␜b�␒␂␌�|␃r�␂$�o�␌�n�␀␄l ␃��s�p��$␛�␌��j16078377 Station configuration is: hsync
mode : sta(84:0d:8e:a7:84:44)
add if0
16086758 mode: 0 -> 3

scandone
state: 0 -> 2 (b0)
state: 2 -> 3 (0)
state: 3 -> 5 (10)
add 0
aid 9
cnt 

connected with hsync, channel 11
dhcp client start...
16243552 connect to ssid hsync, channel 11

ip:192.168.2.28,mask:255.255.255.0,gw:192.168.2.1
19105045 ip:192.168.2.28,mask:255.255.255.0,gw:192.168.2.1
19105349 

Let's publish message now. Memory free=49864
Connecting to mqtt://192.168.5.2:555/
pm open,type:2 0
MQTT Broker Unreachable!!
Let's publish message now. Memory free=48968
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=48504
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=48080
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=47680
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=47200
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=46760
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=46520
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=46240
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=45984
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
...
Let's publish message now. Memory free=2456
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=2200
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=1968
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=1688
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=1432
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=1200
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=920
Connecting to mqtt://192.168.5.2:555/
MQTT Broker Unreachable!!
Let's publish message now. Memory free=664
Connecting to mqtt://192.168.5.2:555/
E:M 136
E:M 328
state: 5 -> 2 (1c0)
rm 0
pm close 7
1874188867 disconnect from ssid hsync, reason 1

reconnect
state: 2 -> 0 (0)
scandone
state: 0 -> 2 (b0)
state: 2 -> 3 (0)
state: 3 -> 5 (10)
add 0
aid 9
cnt 

connected with hsync, channel 11
E:M 1200
dhcp client start...
E:M 440
1875347370 connect to ssid hsync, channel 11

E:M 520
pm_err,flash_tmp alloc fail

Done on Sming 3.8.0 on an ESP12-F.

slaff commented 5 years ago

I guess the following is happening:

Serial.println(system_get_free_heap_size());
mqtt.publish("main/frameworks/sming", "Hello friends, from Internet of things :)");

The queue gets slowly filled with messages to be sent. All of them are waiting for a connection and thus requiring memory. At some point the memory just is not enough to hold all of them and thus your ESP12-F runs out of memory.

There are multiple ways how to solve this. One of them can be that we put a limit on the number of messages, or the total size in bytes of the messages, that can wait in the queue when the client is not connected. This way the client can accept, for example, max 3 messages and discard the rest. mqtt.publish will return true for the accepted and false for the discarded. This is something that we can add in the framework. Another thing that can be done in the application code is to push message in the queue only when the client is connected.

If we go for the items size or items memory limit in a queue then we should apply this also to the SmtpClient, HttpClient and others that queue items before sending them.

verybadsoldier commented 5 years ago

Thanks for looking into it but as statetd in the post I removed the call to publishMessage. So in my opinion there is no publishing taking place anymore.

And sorry for the screwed up indetation in the code which really didn't improve readbility :(

slaff commented 5 years ago

So in my opinion there is no publishing taking place anymore.

That code should be compilable and runnable also in the Host Emulator. That means that you can yourself analyze the memory usage using valgrind. The latter will catch easily any memory issues. Or wait for me but that can take quite some time...

slaff commented 5 years ago

So in my opinion there is no publishing taking place anymore.

Every connection sends a MQTT connect message. Those messages start piling up in the queue. @mikee47 Provided a simplified fix in PR #1742. Give it a try and tell us if it works for you.