Memory leak in WString / TextString Library?

I recently started using the WString.h library for string manipulation as it greatly simplifies many of the difficulties. My latest project was working perfectly until I added WString. Basically, it was going bonkers after the same number of loops. I thought I may have simply run out of space, and switched to a 328. The problem persisted! It did however, make it a few more loops so memory seemed to be the likely cause. Basically I tracked it back to something with WString, but I'm not quite sure what. I was able to make a small example sketch that locks up.

#include <WString.h>

//Program Variables
String lineBuffer = String(50);
int i = 0;

void setup()
{

  Serial.begin(9600);

}

void loop()               
{

  lineBuffer = " This_Is_A_Test!!";
  lineBuffer.append(i);
  lineBuffer.append("00.00  ");
  Serial.println(lineBuffer.substring(1, 17));
  Serial.println(lineBuffer.substring(17, 23));
  delay(10);
  if (i < 9)
    i++;
  else
    i=0;

}

Obviously this should just keep printing over and over, but this is as far as it gets.

This_Is_A_Test!!

000.00

This_Is_A_Test!!

100.00

This_Is_A_Test!!

200.00

This_Is_A_Test!!

300.00

This_Is_A_Test!!

400.00

This_Is_A_Test!!

500.00

This_Is_A_Test!!

600.00

This_Is_A_Test!!

700.00

This_Is_A_Test!!

800.00

This_Is_A_Test!!

900.00

This_Is_A_Test!!

000.00

This_Is_A_Test!!

100.00

This_Is_A_Test!!

200.00

This_Is_A_Test!!

300.00

This_Is_A_Test!!

400.00

This_Is_A_Test!!

500.00

This_Is_A_Test!!

600.00

This_Is_A_Test!!

700.00

This_Is_A_Test!!

800.00



h0

This_Is_A_Test!!

hh00.0

This_Is_A_Test!!

000.00

This_Is_A_Test!!

100.00

This_Is_A_Test!!

200.00

This_Is_A_Test!!

300.00

This_Is_A_Test!!

400.00

This_Is_A_Test!!

500.00

This_Is_A_Test!!

600.00

This_Is_A_Test!!

700.00

You can also see that there is some strange characters in there.

Any help is greatly appreciated.

Seems to me you are just hitting the end of SRAM. Code does weird things when RAM starts thrashing.

I wouldn't call this a leak if you keep allocating memory and not releasing it.

Does append() return anything of interest? I'm too lazy to look it up, but you might see it failing to allocate the space.

Thanks for responding! While I agree that I seem to be hitting the end of SRAM, the way that the library is supposed to work it should not be happening. And it should really never get passed the 50 characters that it was declared with. On each pass of the loop the first line should wipe the array and set it to " This_Is_A_Test!!" thereby not taking up any more memory.

I have poked around in the library code (it is a core library included with the IDE). Unfortunately, I am not familiar enough with c++ to figure out the problem. Hence, I was hoping one of the several c++ guru's we have would take a peek :wink:

Digger, there is a leak in TextString. There is no destructor to clean up allocated memory. However, I believe the powers that be are aware of this.

Mikal

Thanks for the info Mikal. Hopefully it will get straightened out as it is a very useful library.

I've been having some of the same problems using TextString so it's nice to see I'm not the only one having some issues with it.

I use the substring function and my program would loop once then just stop running. Decreasing the size of the strings used would let the program loop a few more times but then it would just stop again. I thought of switching to a 328 but Digger450's experience with it confirms what I thought would happen.

I'm interested to hear about any updates to TextString or workarounds for this.

If you add a destructor to String.h, this should improve things greatly:

    String(const int length = 16);
    String(const char* bytes);
    String(const String &str);
    ~String() { delete [] _array; } // <--- add this line

Mikal

I get an error when I add that line:

o: In function `~String':
D:\Downloads\Arduino\arduino-0015/hardware\libraries\String/WString.h:34: undefined reference to `operator delete[](void*)'

Oops. My mistake.

I mean

   ~String() { free(_array); } // <--- add this line

[edit] Or, if that doesn't work (because free not available in .h) add to .h

 ~String();

and to .cpp

String::~String() { free(_array); }

Mikal

Wow, Mikal, thank you! I'm at 20 minutes plus with no problems on the example sketch. Previously, it would lockup in a few seconds! Your first suggestion worked:

   ~String() { free(_array); } // <--- add this line

Hopefully, we can get this added into the next release. Thank you so much for taking the time to look into this! :slight_smile:

Adding that line worked for me as well. Thanks!

Thanks Mikal. I am visiting family at the moment, with no Arduinos handy to test, but when I get back to work, I'll add this to the String lib.

FWIW, though, this library is not part of the core distribution, to my knowledge. It isn't in the svn, and isn't in the distribution, though that'd be nice. There are also some optimizations I'd like to get to as well, but the day job keeps getting in the way :slight_smile:

Thank you for this fix!

It took me about 2 weeks to figure out why my sketch was locking up after a few minutes.

I finally tracked it down to being out of RAM, which was caused by this leak in WString.

Thanks again, the fix worked fine for me.

Thanks for the fix! However, adding the destructor to Wstring.h breaks my compiler. This gets barfed up, I assume from the linker:

o: In function `main':
undefined reference to `__dso_handle'

Removing the destructor from Wstring.h clears up the error, but I'm left with the memory leak. My sketch is driving a VFD module, and I am implementing a virtual display shift function in software since there is no hardware support. I am creating a new String for every shift (going for a rtl scroll effect). The leak fills up my SRAM within a few seconds freezing the Arduino.

I've done quite a bit of googling and information about this error is sparse. A few results said it had something to do with using ld for linking, criticizing it, and recommending I link with gcc instead. I have no idea how to make that happen. Another suggestion was found in a comment following a dso_handle definition:

// Versions of gcc/g++ after 3.0 (approx.), when configured for Linux
// native development (specifically, --with-__cxa_enable), have
// additional dependencies related to the destructors for static
// objects. When compiling C++ code with static objects the compiler
// inserts a call to __cxa_atexit() with __dso_handle as one of the
// arguments. __cxa_atexit() would normally be provided by glibc, and
// __dso_handle is part of crtstuff.c. Synthetic target applications
// are linked rather differently, so either a differently-configured
// compiler is needed or dummy versions of these symbols should be
// provided. If these symbols are not actually used then providing
// them is still harmless, linker garbage collection will remove them.

void
__cxa_atexit(void (*arg1)(void*), void* arg2, void* arg3)
{
}
void*   __dso_handle = (void*) &__dso_handle;

I have tried adding the dummy symbols, but it throws a multiple references to __dso_handle error. Either it's undefined, or defined more than once. :-?

I am lost. I suspect a misconfigured/miscompiled gcc, but I just don't know. I haven't had an issue until now.

I am using Arduino 0017. I run stock openSUSE 11.0, with stock gcc packages from the main repos. I noticed that gcc and avr-gcc are reporting two different versions of gcc. Could that be a problem? Both packages appear to be current; I am subscribed to the main update repo and there are no updates available.

gcc -v: gcc version 4.3.1 20080507 (prerelease) [gcc-4_3-branch revision 135036] (SUSE Linux)
avr-gcc -v: gcc version 4.3.3 [gcc-4_3-branch revision 144878] (SUSE Linux)

Any ideas? Thanks.

Thank you so much mikalhart. I wonder why there is no updated relase for the textstring lib available for download. That would have spared a lot of time for me ::slight_smile:

I just posted version 0.7 of the String lib, which is basically the bug fix Mikal Hart posted. Sorry for the long delay. String is not part of the core, so it's always been a side project for me, and one I only get to once in a blue moon. The memory bug,at least, should now be gone. Thanks Mikal.

This is an interim version. Hernando Barragan made some nice API changes for a Wiring version, but they change the core libraries a bit, and have not yet been ported over fully. There are also some changes I want to make, because there's some nomenclature and behavior I'm not happy with. Xiaoyang Feng is working on this. The major issue he's working on is to make the library portable without need to modify the core, and to reduce memory usage somewhat. If anyone wants to jump in on this task, mail me.

I just posted version 0.7 of the String lib

Where might I find the link/posting for this new version?

Thanks;
Lefty

Lefty, it looks like it's linked off of String() - Arduino Reference.

The page history shows it was updated today, and the WString.h file in the .ZIP file reflects version 0.7.

Destructor need inicializate vars:

String::~String() { 
      free(_array); 
      _capacity = 0;
      _length = 0;   
}

http://www.megaupload.com/?d=7TAT552N

Do you lost memory with substring?

Many thanks for this thread. It has helped a very frustrating problem!

But once I place the destructor in my code (or download the new zip...same thing), it doesn't function the same any more. Here's the relevant code:

void somefunction()
{
  char* timestamp = isoTimestamp();
  
  Serial.println(timestamp);
}


// iso8601 timestamp of current time
char* isoTimestamp(){
  Serial.println("in isoTimestamp");
  time_t t = now();

  Serial.println("Allocating String(20)");
  String str = String(20);

  Serial.println("Formatting timestamp");
  str.append(year(t));
  str.append("-");
  if(month(t) < 10) str.append("0");
  str.append(month(t));
  str.append("-");
  if(day(t) < 10) str.append("0");
  str.append(day(t));
  str.append("T");
  if(hour(t) < 10) str.append("0");
  str.append(hour(t));
  str.append(":");
  if(minute(t) < 10) str.append("0");
  str.append(minute(t));
  str.append(":");
  if(second(t) < 10) str.append("0");
  str.append(second(t));
  str.append("Z");

  Serial.print("isoTimestamp: ");
  Serial.println(str);
  
  char* outputChars = str.getChars();

  Serial.print("char isoTimestamp: ");
  Serial.println(outputChars);

  return str;
}

The above prints:
in isoTimestamp
Allocating String(20)
Formatting timestamp
isoTimestamp: 2010-06-22T23:33:23Z
char isoTimestamp: 2010-06-22T23:33:23Z
+?10-06-22T23:33:23Z

Note the two first bytes in the returned char* are wrong/odd (it isn't really a '?', but it was interpreted as a newline in the forum post). This occurs whether I "return str;" or "return str.toChars();"

I'm still pretty new to Arduino, and haven't messed with C-based stuff (aka pointers) since college long ago, so hopefully this is just a newbie mistake. But it worked fine before adding the String destructor. Well, other than the memory leak, anyway! Can someone help?

Thanks in advance.