Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak when connect() #673

Open
edodad opened this issue Oct 29, 2019 · 7 comments
Open

Memory leak when connect() #673

edodad opened this issue Oct 29, 2019 · 7 comments

Comments

@edodad
Copy link

edodad commented Oct 29, 2019

Hi
following the sample, I noticed that the library uses 1KB aprox of heap every time I call connect()

so for example:

void loop()
{
...
if ( !pubSubClient.connected()) {
mqttConnect();
}
...
}

and

void mqttConnect()
{
...
if (pubSubClient.connect(clientid, user, password)) {
...
}
else
{
...
}
...
}

It means that every time my router lost tinternet connection, pubSubClient.connected() is FALSE, mqttConnect() is called and THEN it butns 1KB of heap every time, without releasing the OLD memoty

How can I solve the issue?

Thannks

@edodad
Copy link
Author

edodad commented Oct 29, 2019

Arduino Core 2.5.2
pubsubclient 2.7.0

@Speedy2k
Copy link

I seems to have a similar issue but on the long run, it seems to be after around 24 hours of running, i don't know why, it try to reconnect and it jam in the connect function and never get out of there, i don't exactly know what is happening since i don't get any error, the watchdog on my esp32 doesn't kick. Everything just stop!?

@knolleary
Copy link
Owner

The PubSubClient code is very stable and has not changed for some time. No-one else has seen this issue, so there must be something about your sketch, or the particular network client you are using.

Looking at your code (which you've sent me via email) to see where you are allocating additional memory each time it connects I can see you are creating a new instance of the WiFiClientSecure and PubSubClient objects every time you try to reconnect. Similarly you generate a new client ID every time. You could try statically assigning them at the start of the script rather than recreating them every time - this is the approach all of the example sketches take.

@TD-er
Copy link

TD-er commented Nov 9, 2019

Well, I have some reports of users with poor WiFi connections, stating when MQTT (thus this library) is used, they run out of memory and thus crash. (example: letscontrolit/ESPEasy#2684 )

About recreating the WiFiclient. That's also what I'm doing and was suggested here: esp8266/Arduino#4497 (comment)

So I will also have a look at the progress of this issue.

@narenayak
Copy link

narenayak commented Aug 10, 2020

Newbie so plz correct me if I am wrong to comment on this issue it may be a different issue but is related to esp32 and connect().
uncommented one new code where i had to use a flag instead of mqttClient.connect() in an if/else.My problem was solved by doing this
Commented part indicates what I was doing earlier .
The problem I was facing on esp32 hardware was is as mention below :
Wifi will be available on boot.Broker not available.
At this point when it enters the if (mqttClient.connect(xyz) ) i disconnected the wifi and my esp32 used to hang here, never used to come out of it.Basically I had to use external reset or power recycle. i tried added a wifi available flag (network_connected_flag()) which never helped.
Other details:
SDK Version:v3.3-beta1-506-gebdcbe8c6
IDF Version:v3.3-beta1-506-gebdcbe8c6
pubsubclient: 2.8
Arduino 1.8.5

if ((mqtt_user != "" && mqtt_passwd != "" && mqttClient.connect((char*) hostName.c_str(), (char*)mqtt_user.c_str(), (char*)mqtt_passwd.c_str(), (char*)pubTopic.c_str(), 0, 0, (char*)dynamic_will_msg.c_str())) && network_connected_flag()) //added network_connected_flag()on 07/08/2020
  {
   Serial.println("MQTT Connected via username & password");
   portENTER_CRITICAL(&timerMux);
   cTime =0;
   portEXIT_CRITICAL(&timerMux);
   configured_flag=1;
   mqttClient.publish((char*)pubTopic.c_str(),(char*)dynamic_connected_msg.c_str());
   mqttClient.subscribe((char*)subTopic.c_str());
   return true;
  }
  else if((mqttClient.connect((char*) hostName.c_str(),(char*)pubTopic.c_str(), 0, 0, (char*)dynamic_will_msg.c_str())) && network_connected_flag())  //added network_connected_flag()on 07/08/2020
  {
   Serial.println("MQTT Connected via no username & password");
   portENTER_CRITICAL(&timerMux);
   cTime =0;
   portEXIT_CRITICAL(&timerMux);
   configured_flag=1;
   mqttClient.publish((char*)pubTopic.c_str(),(char*)dynamic_connected_msg.c_str());
   mqttClient.subscribe((char*)subTopic.c_str());
   return true;
  }

Solved it by using a flag instead of trying to connect it in the if loop , I am not sure if this method is correct.Please correct me if i am wrong . As of now it is working fine probable i will test it this week and post if there is some problem.

boolean connectMQTT() 
{  
   //Serial.println("Entering MQTT Connect");
   bool mqttclient_connected_flag;
   String device_id =hostName;
   //device_id.replace("-","");
   String dynamic_will_msg=deviceName+"--"+mqtt_will_msg;
#ifdef STANDARD_TIME
   //String dynamic_will_msg = "{\"state\": {\"reported\":{\"Client ID\":\""+device_id+"\",\"Device Name\":"+"\""+deviceName+"\""+",\"Status\":\"Disconnected\"}}}";
   String dynamic_connected_msg = "{\"state\": {\"reported\":{\"Client ID\":\""+device_id+"\",\"Device Name\":"+"\""+deviceName+"\""+",\"Status\":\"Connected\","+sG_Present_Timestamp_std+","+S_bootreason+","+S_resetreason[0]+","+S_resetreason[1]+",\"SWRebootCause\":"+sw_reset_reason+"}}}";
#elif defined  EPOCH_TIME
  //String dynamic_will_msg = "{\"state\": {\"reported\":{\"Client ID\":\""+device_id+"\",\"Device Name\":"+"\""+deviceName+"\""+",\"Status\":\"Disconnected\"}}}";
  String dynamic_connected_msg = "{\"state\": {\"reported\":{\"Client ID\":\""+device_id+"\",\"Device Name\":"+"\""+deviceName+"\""+",\"Status\":\"Connected\","+sG_Present_Timestamp_epoch+","+S_bootreason+","+S_resetreason[0]+","+S_resetreason[1]+",\"SWRebootCause\":"+sw_reset_reason+"}}}";
#endif
   if (mqttClient.connected() && network_connected_flag()) //added network_connected_flag()on 07/08/2020
   {
    mqttClient.loop();
    mqtt_retries =0;
    portENTER_CRITICAL(&timerMux);
    cTime =0;
    portEXIT_CRITICAL(&timerMux);
    configured_flag=1;
    return true;
   }
   else
   {
    mqtt_retries++;
     if(mqtt_retries > MAX_MQTT_RETRY)
       {
        Serial.print("Going to reboot as mqtt failed for more than 15 times....."); 
        timerAlarmDisable(timer_one);
        Serial.print("Reason code written ");
        delay(10);
        esp_restart(); 
       }
    }
   Serial.print("Connecting to MQTT Broker with Client ID:");
   Serial.println(hostName);
   Serial.print("Connecting using user Name::Password::Will Topic::Will msg with Client ID:");
   Serial.println("--"+mqtt_user+"::"+mqtt_passwd+"::"+pubTopic+"::"+dynamic_will_msg);
   
   if (mqtt_user != "" && mqtt_passwd != "")
   {
    Serial.println("MQTT Connecting via username & password");
    mqttclient_connected_flag = mqttClient.connect((char*) hostName.c_str(), (char*)mqtt_user.c_str(), (char*)mqtt_passwd.c_str(), (char*)pubTopic.c_str(), 0, 0, (char*)dynamic_will_msg.c_str()) & network_connected_flag();
    Serial.println(mqttclient_connected_flag);
   }
   else
   {
    Serial.println("MQTT Connecting via no username & password");
    mqttclient_connected_flag = mqttClient.connect((char*) hostName.c_str(),(char*)pubTopic.c_str(), 0, 0, (char*)dynamic_will_msg.c_str()) & network_connected_flag();
    Serial.println(mqttclient_connected_flag);
   }

  if (mqttclient_connected_flag) //added network_connected_flag()on 07/08/2020
  {
   Serial.println("MQTT Connected");
   portENTER_CRITICAL(&timerMux);
   cTime =0;
   portEXIT_CRITICAL(&timerMux);
   configured_flag=1;
   mqttClient.publish((char*)pubTopic.c_str(),(char*)dynamic_connected_msg.c_str());
   mqttClient.subscribe((char*)subTopic.c_str());
   return true; 
  }
  else
  {
    Serial.print("MQTT not Connected, rc=");
    Serial.print(mqttClient.state());
    Serial.print(", retry count is ");
    Serial.print(mqtt_retries);
    Serial.println(", waiting 120 seconds to retry");
    sw_reset_reason="\"mrc="+String(mqttClient.state())+"\""; //added on 07/08/2020
    portENTER_CRITICAL(&timerMux);
    configured_flag=0;
    portEXIT_CRITICAL(&timerMux);
    disconnectMQTT();
    //delay(500);
    Debugln("DEBUG:Configuration Flag LOW");
    if(mqtt_retries> (MAX_MQTT_RETRY-1)) //added on 07/08/2020
    {
    Serial.print("Saving mqtt failed status to memory...."); //added on 07/08/2020
    saveConfig() ? Serial.println("sucessfully.") : Serial.println("not succesfully!");; //added on 07/08/2020
    }
    return false;
  }
  //configured_flag=0;
  //Serial.println("Configuration Flag LOW");
  //disconnectMQTT();
  //delay(500);
  //return false;
}
bool network_connected_flag()
{
 
//#ifdef  OnWiFi  
  if(WiFi.status() == WL_CONNECTED )
  {
    return true;
  }
  else
  {
    return false;
  } 
}

@TD-er
Copy link

TD-er commented Aug 10, 2020

Can you use 3 backticks at begin and end of a code block?
You're now using only 1 backtick, which is meant for inline code.

@narenayak
Copy link

Can you use 3 backticks at begin and end of a code block?
You're now using only 1 backtick, which is meant for inline code.

Thx corrected it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants