Watchdog called when freememory() = 952, best way to investigate

I have recently made a number of changes to a large application that was stable running for a long period with no issues. Recently the watchdog has been kicking in and recording the current state, saving that to eeprom then doing a System.reset();

It appears to be in a repeating pattern of running for just over 2 hours and then being caught by the watchdog. At this point the free memory is 952. Clearly there is an issue with free memory.

Can anyone offer advice about the best approach to track down the cause of the memory leakage?

@armor, when you say “watchdog”, do you mean the Application Watchdog? Here are some obvious questions regarding free memory:

  • Do you use Strings?
  • Do you use std:: functions that use dynamic memory allocation?
  • Do you create and destroy objects dynamically?

What is the initial free memory when your code first runs?

@peekay123, That was super quick! Correct - Application Watchdog setup like this - ApplicationWatchdog wd(MCHK, restartResume); There is also a wd.checkin(); //check-in with the watchdog every loop. Any delays are handled by a routine which calls waits 1 millsecond then calls Particle.process();

I have a flag to ensure Particle.connect(); is only called once per session.

I do use strings and in particular one called dataStr which is used to construct JSON valid event messages. This string space is allocated in setup() with dataStr.reserve(MAXDATA); //reserve string size to avoid heap data issue

I am not aware of any std:: functions and I don’t think I am create and destroying objects dynamically although I can now see another potential issue with a wake from low power sleep that re-runs device driver object.begin() routines. However, in this case sleep mode was not entered.

Thus, probably it is the dataStr string because I have recently added variables to the JSON structure. If I put more than the MAXDATA number of characters in dataStr repeatedly every minute as I am then it will use up memory! Probably solved.

@armor, Strings are nefarious that way. Every time you beyond the String size, it will allocate another block of storage (16 bytes I believe). When you say it is allocated in setup(), I assume you mean it is global ins scope and sized in setup().

What you don’t see with Strings is all the temporary Strings created during concatenations, parsing and other functions. THESE will create the fragmentation you want to avoid. I bet that if you print out freeMemory() before and after your JSON operations, you will find it steadily declines.

If you want to build a robust system, you should seriously consider converting to c-strings (aka char arrays). A little more work is required but they won’t have any heap surprises. You can also use snprintf() to build your JSON strings as well. :wink:

1 Like

@peekay123, Thanks on that advice - it tallies up now, if my messages were 335 bytes and 16 bytes was added to that each time then I can see how over a 2 hour period with a message sent every minute that memory would be eaten up.

Another thought. In the Particle Reference documents it states that optional data (up to 255 bytes) yet I have been publishing 335 bytes and that has been working. Does that mean 255 is not really a limit or has that limit been increased and the documents haven’t?

@armor,

If you "recycle" the String (eg. string = "":wink: then it will only grow when you again exceed its size. Otherwise it will not grow. As I indicated, the fragmentation may come from the transient Strings created during your JSON construction.

Are you sure your are publishing 335 bytes? Where are you getting that count?

@peekay123, Good point - the actual optional data string is this (335 characters): "{\"date\":\"2017-12-08T12:38:54\",\"VR\":[{\"N\":\"C\",\"V\":\"1\"},{\"N\":\"R\",\"V\":\"1\"},{\"N\":\"U\",\"V\":\"1\"},{\"N\":\"Z\",\"V\":\"101\"},{\"N\":\"D\",\"V\":\"0\"},{\"N\":\"Q\",\"V\":\"-72\"},{\"N\":\"Y\",\"V\":\"51.529305\"},{\"N\":\"X\",\"V\":\"-1.118472\"},{\"N\":\"ZZ\",\"V\":\"33.000000\"},{\"N\":\"AO\",\"V\":\"0\"}]}". However, I needed to take off the 83 ‘’ so actually 252 and just inside the limit!

I have had a look at snprintf() as you suggested and it returns the string length created, just what I need to do this safely.

@armor, it not only returns the number of chars “printed” but you also specify the max size to be printed (255) so it adds protection as well. :smile:

2 Likes

Actually it's an exponential growth - starting with 16 byte, each resize will allocate double the current size and relocate the string leaving the previous spot hanging around.

2 Likes

Thanks @ScruffR! That’s even worse than I thought!

Hi @ScruffR, @peekay123, can I trouble you to clarify the following, please, on the handling of String allocation.
I’ve avoided String types in my main/global context, but do instantiate several small ones (<200 chars) in a function call that parses an incoming command string (passed in as a char*), as they save a fair bit of code writing.
If I am only declaring them in the function, do all their allocations end up on the stack, and get safely discarded on function exit? It seemed to me this is how it ‘should’ work but, in light of the discussion above, heap allocation would also seem possible.
Cheers,
AT

1 Like

No. Objects get instantiated off-side the stack. Only the object pointers are on the stack. And in this case the String content does actively get allocated on the heap by the Object itself.
For local String objects even the preassigned size doesn't really help avoiding heap fragmentation.

2 Likes

Cheers, @Scruffr. Looks like I should follow @peekay123’s advice above, and refactor that code to use char arrays, if I want a stable app. Meantime, I’ll put in a heap freemem() check to see if I have a problem developing over time. Thanks, gents.

2 Likes

@ScruffR, having changed my code to use char* and char arrays - I am getting a constant memory leakage from one small are of code that I didn’t have a problem with when using String!

Original code was:
String connectedSSID = “xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx”; //WAP name maximum size 32 char

                if (!connectedSSID.equals(String(WiFi.SSID())))
                {
                    requestPublicIP();
                    isWAPChanged = true;
                }
                connectedSSID = String(WiFi.SSID());                //refresh the WAP name 

New code is as follows -
char* connectedSSID; //WAP name changed from String object to char*
char conSSID[33]; //maximum SSID length is 32 char plus \0 terminator
char chkSSID[33]; //char array for receiving current SSID
char* chkSSIDptr; //define a char* and make it point to start of chkSSID when used to receive current SSID

Code:
chkSSIDptr = chkSSID;
chkSSIDptr = strdup(WiFi.SSID());
if (strcmp(connectedSSID, chkSSIDptr) != 0)
{
requestPublicIP();
isWAPChanged = true;
}
connectedSSID = strdup(WiFi.SSID());

The first strdup() leaks 24 bytes and so does the second one. That is measured by System.freeMemory() before and after

I’d also stay away from strdup() as it does just the same thing as String objects - it allocates memory on the heap, eventually causing heap fragmentation.
But the bigger issue here is that you don’t free the allocated memory anymore.

strdup() uses malloc() to allocate an area big enough to hold the string and copies the contents of the original string over and hands you back the pointer to that allocated memory.
But when you’re done with that string it’s up to you to call free(chkSSIDptr) and free(connectedSSID) otherwise you’ll lose the reference to that memory next time you redirect the pointer.

I’d rather go for a char[] and use strcpy() or strncpy() to copy the string over.

BTW, what are conSSID[33] and chkSSID[33] for? You don’t seem to use them.
After strdup() the pointers you intended to point there won’t point there anymore but rather at the heap portion allocated.

You can - and I would - use this instead

  strncpy(chkSSID, WiFi.SSID(), sizeof(chkSSID)-1);
2 Likes

@ScruffR, I looked at StackOverflow for advice on taking the output/result of WiFi.SSID() which is a const char* to char* - I didn’t look into exactly how it works and clearly it is allocating memory that I am then not releasing.

conSSID[33] is the allocation of memory to hold the actual char string of the connected SSID. This gets used elsewhere. In an earlier version I had not allocated globally any space for the output/result of WiFi.SSID(). Rather, I had defined the char* chkSSIDptr; just before it was used. chkSSID[33] was an allocation of memory for this use as I wondered whether that was where the memory leakage was happening. When I had tried char* chkSSIDptr = WiFi.SSID(); the compiler had complained of an invalid conversion from const char* to char*.

I have used strncpy() as you suggested in place of strdup() and the memory leakage is fixed. Thanks

3 Likes