Memory leak in StringHolder

General JUCE discussion
1

Hi there,

I think there is a problem in the StringHolder::createUninitialisedBytes function because of the StringHolder object "s" is never deleted... Its internal text is returned but the object itself is left alive.

static CharPointerType createUninitialisedBytes (size_t numBytes)
{
     numBytes = (numBytes + 3) & ~(size_t) 3;
     StringHolder* const s = reinterpret_cast<StringHolder*> (new char [sizeof (StringHolder) - sizeof (CharType) + numBytes]);
     s->refCount.value = 0;
     s->allocatedNumBytes = numBytes;
     return CharPointerType (s->text);
}

 

2

No, there's no mistake in that code. It's gnarly, but correct.

If you're seeing a leak that seems to be from StringHolder, then it's most likely because you're leaking a string yourself. Probably a static String you've declared somewhere.

3

It's leaking from the following source line:

infoLabel_.setText(TRANS(L"Move the mouse over an object to get info"),
                   juce::dontSendNotification);
4

It's leaking from the following source line:

infoLabel_.setText(TRANS(L"Move the mouse over an object to get info"),
                   juce::dontSendNotification);
5

Agh, why are people still using that dratted 'L' prefix!? There is nowhere where it's helpful in any way at all, and in situations like this it'll cause an extra allocation of a useless temporary string object! See my coding standards doc for more info about literal strings!

But no, that line will not cause a leak. It may cause something else show on the leak detector, e.g. maybe you're not cleaning up the translation stuff before shutdown. I don't know, but honestly, you're looking in the wrong place.

6

We're using the 'L' prefix because of internationalization and the need to use Asian characters, and so UTF-16.

For now we're manipulating wstring but a plan to move to u16string is on the rail.

7

I think that's a really bad decision - the only string literal encoding that works the same in all compilers and editors is ascii + utf8 escape characters. See http://www.juce.com/documentation/coding-standards

8

If we only considered the C++98 standards, then I would be totally agree.

But we're working using the C++11 standards, and therefore the u16string is strictly defined.

9

I'm not talking about the code that handles these strings - obviously there are classes that will correctly handle any format you want to use. I'm talking about embedding string literals in your source code in a way where the encoding type won't be mis-interpreted by different compilers and editors.

10

Ah right... Yeah. The source code is written using UTF-16 encoding of the source files...

And we're using compilers that well-recognized them so... Internally we don't have any issues.

11

Ok, well, I still think that's a bad policy, but good luck with it!

But if you ignore all of my other advice, please at least do this: Only use 'L' for strings that actually contain non-ascii characters. Using it like you did above for a plain ascii string incurs some completely useless overhead - wasting both speed and code size.