Pages: [1] 2   Go Down
Author Topic: Memory leak in WString / TextString Library?  (Read 3641 times)
0 Members and 1 Guest are viewing this topic.
0
Offline Offline
Sr. Member
****
Karma: 0
Posts: 375
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.

Code:
#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.

Code:
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.
Logged

Canada
Offline Offline
Full Member
***
Karma: 0
Posts: 246
Code Monkey
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

I yield() for co-routines.

0
Offline Offline
Sr. Member
****
Karma: 0
Posts: 375
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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  smiley-wink
Logged

Austin, TX USA
Offline Offline
God Member
*****
Karma: 4
Posts: 997
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

0
Offline Offline
Sr. Member
****
Karma: 0
Posts: 375
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

0
Offline Offline
Newbie
*
Karma: 0
Posts: 2
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Austin, TX USA
Offline Offline
God Member
*****
Karma: 4
Posts: 997
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

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

Mikal
Logged

0
Offline Offline
Sr. Member
****
Karma: 0
Posts: 375
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I get an error when I add that line:

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

Austin, TX USA
Offline Offline
God Member
*****
Karma: 4
Posts: 997
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Oops.  My mistake.

I mean

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

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

Code:
~String();

and to .cpp

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

Mikal
« Last Edit: May 11, 2009, 10:35:10 pm by mikalhart » Logged

0
Offline Offline
Sr. Member
****
Karma: 0
Posts: 375
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:

Code:
  ~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!   smiley
« Last Edit: May 12, 2009, 12:02:41 am by Digger450 » Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 2
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Adding that line worked for me as well. Thanks!
Logged

New York, NY, USA
Offline Offline
Newbie
*
Karma: 0
Posts: 40
Howdy. I don't check PMs often, so email me!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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 smiley
Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 1
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 1
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Code:
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:

Code:
// 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.

Code:
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.
Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 3
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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  :smiley
« Last Edit: March 28, 2010, 03:08:44 pm by saibot » Logged

Pages: [1] 2   Go Up
Jump to: