Memory leaks with class members

Question

I don't think this requires another question, so I'm editing this extremely relevant one.

I had a code that would draw text by converting from char* to wchar_t*, and I suspected a memory error, as the program memory would go up as a crazy pace (5,000 K to 1,500,000 in minutes).

I suspected mbstowcs(), but I think I've found the problem now.

I am using a rather bad way of getting colors to draw things in general.

class MainClass {
    public:
        ID2D1SolidColorBrush* custom_color;
        ID2D1SolidColorBrush  get_rgba(float r, float g, float b, float a) {
            // render is a validated ID2D1RenderTarget*
            render->CreateSolidColorBrush(D2D1::ColorF(r,g,b,a),&custom_color);
            return custom_color;
        }
};

The memory usage increase is almost assuredly coming from this function. Are there any better ways I could return custom colors like this?

Whats the output of `valgrind --leak-check=full ./program` ? — Galimov Albert, Mar 01 '13 at 07:33
This is being run on VS2012, I am not too experienced with parameters to the debugger, even less the console.. — jtst, Mar 01 '13 at 07:38

selbie · Answer 1 · 2013-03-01T07:37:41.160

1

You don't appear to actually have a memory leak. And many "leaks" reported by memory leak tools are sometimes false positives. However, there's one easy fix to eliminate nxtx as a possible leak. Since you are allocating a fixed amount (250 chars) each time, you can easily allocate this off the stack:

    const int MY_MAX_STRING_SIZE = 1000;
    wchar_t ntxt[MY_MAX_STRING_SIZE]; // simple stack allocation
    mbstowcs(ntxt,text.c_str(),MY_MAX_STRING_SIZE);
    ntxt[MY_MAX_STRING_SIZE-1] = 0; //insure null termination
    render->DrawTextA(ntxt,text.length(),font,trect,color);
}

One thing I noticed is that your mbstowc call is specifying "size" as the max number of chars to copy, but is hardcoding 250 as the length. Are you certain you certain that "size" is always less than 250?

edited Mar 01 '13 at 07:37

answered Mar 01 '13 at 07:21

selbie

100,020
15
103
173

I might be thinking of the wrong word, when I say memory leak, then. If I watch the program memory usage in the task manager, as this code is being executed at least 50-60 times a second, the memory usage will be nearing 1,500,000 K in about 5 minutes. The memory does not increase when I comment out the lines executing the function. – jtst Mar 01 '13 at 07:25
Task manager is not a memory leak detection tool. It is wildly inaccurate - especially since the CRT will often cache allocation from the system for fast re-allocation later. However if you see your program in task manager steadily rise in memory usage, you likely have a leak somewhere. – selbie Mar 01 '13 at 07:26
Yeah; at first I did not know where the leak was, but after some commenting, it was narrowed down to this function in particular. – jtst Mar 01 '13 at 07:29
Good catch; I've rewritten the code in the post to make a little more sense using the new[] and delete[]. – jtst Mar 01 '13 at 07:32
msbtowcs doesn't guarantee null termination. Initialize ntxt[size-1] to zero after mbtowcs returns. (asserting that size is always >= 1 of course). – selbie Mar 01 '13 at 07:35
And finally, if you know a reasonable maximum for "size", just do the stack allocation like I show above. – selbie Mar 01 '13 at 07:36

score 0 · Answer 2 · answered Mar 01 '13 at 07:24

0

Its not lookikg as you have a memory leak. But there may be dangerous behaiour: mbstowcs accepts max length of wchar_t in dest, but you giving string length. Fix for this:

    int size=text.length()+1;
    enum { NTXT_LEN = 250 };
    wchar_t* ntxt= new wchar_t[NTXT_LEN];
    mbstowcs(ntxt, text.c_str(), NTXT_LEN);

Also, it is worth replacing raw new with something like scoped_array

answered Mar 01 '13 at 07:24

Galimov Albert

7,269
1
24
50

Trying this likely did make it safer, but the program's memory usage still went up at a violent pace. – jtst Mar 01 '13 at 07:30

jtst · Accepted Answer · 2013-03-06T04:20:07.883

0

My problem was in a code that returns an ID2D1SolidColorBrush* as the color variable, which was executing a Create() and not a Release every time it was called, so the brushes would build up and turn into memory leaks.

edited Mar 06 '13 at 04:20

answered Mar 01 '13 at 23:08

jtst

312
4
16

Memory leaks with class members

3 Answers3