Memory leak using std::string

guillaumequest (6)
Hello,
I've already spent two hours to locate my memory leak and now I really don't see how I could fix it.

I'm making a code parser and in this part of the code I detect a variable type (basicly if 'expression' starts and ends with quotes, strip them, create a new string, pass it as a parameter to a new Variable.

I use the Variable class pretty much everywhere in my code without any leaks so I'm pretty sure it isn't a problem with that class.

Here is the code that leaks
1
2
3
4
5
 
if (expression[0]=='"' && expression[expression.length()-1]=='"')  	
    {
        //LEAK!? breaking here solves the leak, and it's not in Variable so there must be a problem here
        return new Variable("tmp","string",new std::string(expression.substr(1,expression.length()-2)));
    }


Other codes such as this one do not seem to leak
1
2
3
4
 
if (canBeParsedAsFloat(expression)) {
        float f=atof(expression.c_str());
        return new Variable("tmp","float",new float(f));
    }


Thanks for your help !
firedraco (6247)
As far as I can tell, there is not problem there. I'll need to see the Variable class.
guillaumequest (6)
sure ! It doesn't do much however, I guess there could be a problem with the treatement of the Variable pointer that's returned by my function, but I don't get any problems when returning a float*, so...

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
 
#include "variable.h"

#include <string>

#include "mesh.h"
#include "terrain.h"

Variable::Variable()
{
    m_null=true;
}

Variable::Variable(std::string name, std::string type, void* value)
{
    m_name=name;
    m_type=type;
    m_uNull=false;
    m_null=false;
    if (value!=0)
        m_value=value;
    else
        m_null=true;
}

Variable::Variable(bool uNull)
{
    m_null=true;
    m_uNull=uNull;
}

Variable::~Variable()
{
    if (!m_null && !m_uNull) delete m_value;
    m_null=true;
}

std::string Variable::getType()
{
    if (m_null) return "";
    else return m_type;
}

std::string Variable::getName()
{
    if (m_null) return "";
    else return m_name;
}

void* Variable::getValue()
{
    if (m_null) return 0;
    else return m_value;
}

bool Variable::null()
{
    return m_null;
}

void Variable::set(void* value)
{
    if (!m_null && !m_uNull) delete m_value;
    m_value=value;
}

void Variable::init()
{
    if (m_type=="Mesh") m_value=new Mesh();
    else if (m_type=="Terrain") m_value=new Terrain();
    else if (m_type=="float") m_value=new float();
    else if (m_type=="string") m_value=new std::string();

    m_null=false;
}

Variable::Variable(Variable const& copy)
{
    if (!m_uNull)
    {
        int b;
    }
}
firedraco (6247)
What happens if you call init after you have new'd a variable? That would cause a leak. (I don't think that's the problem here though.)

My guess is that you are never delete'ing the Variable* you are returning from your function that is making it. Do you have that code?
guillaumequest (6)
Valgrind says :
==21415== 918 bytes in 54 blocks are definitely lost in loss record 387 of 406
==21415== at 0x402569A: operator new(unsigned int) (vg_replace_malloc.c:255)
==21415== by 0x469FD05: std::string::_Rep::_S_create(unsigned int, unsigned int, std::allocator<char> const&) (in /usr/lib/libstdc++.so.6.0.13)
==21415== by 0x46A0E30: ??? (in /usr/lib/libstdc++.so.6.0.13)
==21415== by 0x46A10A1: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::string const&, unsigned int, unsigned int) (in /usr/lib/libstdc++.so.6.0.13)
==21415== by 0x805DC36: evaluate(std::string, LocalSet*) (basic_string.h:2006)
==21415== by 0x805BDB0: parseStatement(std::string, LocalSet*) (parser.cpp:471)
==21415== by 0x805CCBB: evaluateFunction(std::string, LocalSet*) (parser.cpp:346)
==21415== by 0x805DB0C: evaluate(std::string, LocalSet*) (parser.cpp:221)
==21415== by 0x805BDB0: parseStatement(std::string, LocalSet*) (parser.cpp:471)
==21415== by 0x805C540: parseIfStructure(std::string, LocalSet*) (parser.cpp:402)
==21415== by 0x805EFF2: runCode(std::string, LocalSet*) (parser.cpp:43)
==21415== by 0x805FD97: TutorialApplication::update() (TutorialApplication.cpp:58)
==21415== by 0x80517A9: OgreManager::frameRenderingQueued(Ogre::FrameEvent const&) (BaseApplication.cpp:249)
==21415== by 0x4368681: Ogre::Root::_fireFrameRenderingQueued(Ogre::FrameEvent&) (OgreRoot.cpp:829)
==21415== by 0x436956F: Ogre::Root::_fireFrameRenderingQueued() (OgreRoot.cpp:884)
==21415== by 0x43695B4: Ogre::Root::_updateAllRenderTargets() (OgreRoot.cpp:1388)
==21415== by 0x436970F: Ogre::Root::renderOneFrame() (OgreRoot.cpp:966)
==21415== by 0x436977C: Ogre::Root::startRendering() (OgreRoot.cpp:956)
==21415== by 0x8051B52: OgreManager::go() (BaseApplication.cpp:197)
==21415== by 0x805FC94: main (TutorialApplication.cpp:89)


The code I gave was from the evaluate function, and basic_string.h:2006 is :
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
/**
       *  @brief  Get a substring.
       *  @param pos  Index of first character (default 0).
       *  @param n  Number of characters in substring (default remainder).
       *  @return  The new string.
       *  @throw  std::out_of_range  If pos > size().
       *
       *  Construct and return a new string using the @a n characters starting
       *  at @a pos.  If the string is too short, use the remainder of the
       *  characters.  If @a pos is beyond the end of the string, out_of_range
       *  is thrown.
      */
      basic_string
      substr(size_type __pos = 0, size_type __n = npos) const
      { return basic_string(*this,
			    _M_check(__pos, "basic_string::substr"), __n); } //LINE #2006
firedraco (6247)
Hm. I'm not sure...sorry. :/ Maybe you could try replacing those pointers with smart pointers and see if that fixes it.

This is unrelated, but maybe you could use boost::any instead of that variable class?
closed account (z05DSL3A)
What type is m_value? it looks like it might be a void*.
You will have to cast to the correct type to delete a void *. (See edit)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
 
#include <iostream>

class Foo
{
public:
    Foo()
    {
        std::cout << "Foo()" << std::endl;
    }
    ~Foo()
    {
        std::cout << "~Foo()" << std::endl;
    }
};

class Variable
{
public:
    Variable(void * value)
    {
       vPtr =  value;
    }
    ~Variable()
    {
        delete static_cast<Foo*>(vPtr);
    }
    void * vPtr;
};

class Variable2
{
public:
    Variable2(void * value)
    {
       vPtr =  value;
    }
    ~Variable2()
    {
        delete vPtr;
    }
    void * vPtr;
};


void main()
{
    std::cout << "Variable  : vPtr  : ";
    Variable * vPtr = new Variable(new Foo());
    std::cout << "Deleting  : vPtr  : ";
    delete vPtr;
    
    std::cout << "Variable2 : v2Ptr : ";
    Variable2 * v2Ptr = new Variable2(new Foo());
    std::cout << "Deleting  : v2Ptr : ";
    delete v2Ptr; //does not call ~Foo()
    
    std::cout << std::endl;
    return;
}
 
Variable  : vPtr  : Foo()
Deleting  : vPtr  : ~Foo()
Variable2 : v2Ptr : Foo()
Deleting  : v2Ptr :


Edit:
I can't remember what the Standard says about deleting void pointers that address built-in types and your compiler may well handle them properly, but void pointers that address class objects are a problem.
Last edited on
closed account (S6k9GNh0)
I might be wrong but:

1
2
3
4
5
6
7
8
9
 
void Variable::init()
{
    if (m_type=="Mesh") m_value=new Mesh();
    else if (m_type=="Terrain") m_value=new Terrain();
    else if (m_type=="float") m_value=new float();
    else if (m_type=="string") m_value=new std::string();

    m_null=false;
}


You assign m_value a value on Variable construction. During init(), you blindly assign it a new value. I can't see the actual flow of the program so I'm not sure. Also, why not take advantage of templates instead of use void pointers?

Also, is this a serious parsing project? If so, I would definitely look into using something other than std::string for you're parsing needs.
Last edited on
guillaumequest (6)
@firedraco : yeah, I'll see what boost::any can do for me...
@Grey wolf : yes, m_value is a void*. The problem is I need to do a collection of variables. It doesn't seem very comfortable to cast the void pointer to the real type of object, but I guess I have no choice... I'll try this, thanks (btw it's weird I never had any leaks before if the variables didn't get deleted. What does the compiler do when you delete a void pointer ?)
@computerquip : that's true, although the program never does init() after giving a value with the constructor. This definitly breaks encapsulation, so I've fixed it (now init() deletes the pointer before assigning it).
What would you suggest for parsing ? It's not a whole new syntax we need to parse, more of a script-like language...

Thanks everyone !
guillaumequest (6)
From another forum :
"Oh sure, you can delete a void* pointer and on MSVC it generally works fine so long as it's raw memory or built-in types. But when you start deleting void* pointers to classes, you get problems.
I'm kind of betting it's undefined behavior. But in C++ this is very easy to avoid - simple use templates!"

So I guess that's it !
I tested : 
1
2
3
4
5
 
while (true) {
    Mesh* m=new Mesh();
    Variable* var=new Variable("tmp","Mesh",m);
    delete var;
}
which nearly crashed my computer with an enormous leak...

Do you think it would be possible to create a collection that would accept any pointer while keeping its type ?
closed account (z05DSL3A)
guillaumequest wrote:
What does the compiler do when you delete a void pointer ?

I think that technically it is undefined behavior, so it may do anything. It may delete built-in types properly, it may not. If it doesn't know how to delete something it may just set the pointer to null.

standard wrote:
In the first alternative (delete object), if the static type of the operand is different from its dynamic type, the static type shall be a base class of the operand’s dynamic type and the static type shall have a virtual destructor or the behavior is undefined. In the second alternative (delete array) if the dynamic type of the object to be deleted differs from its static type, the behavior is undefined.73)
...
73) This implies that an object cannot be deleted using a pointer of type void* because there are no objects of type void.
5.3.5 Delete, paragraph 3 and footnote

Last edited on
closed account (z05DSL3A)
guillaumequest wrote:
Do you think it would be possible to create a collection that would accept any pointer while keeping its type ?
firedraco has already suggested using boost::any instead of Variable, but you could read "Valued Conversions"[1] by Kevlin Henney (boost::any is based on the contents of this article) for ideas on how to change Variable.


[1] http://www.two-sdg.demon.co.uk/curbralan/papers/ValuedConversions.pdf
guillaumequest (6)
Okay, I'll look at that...
Thanks guys !
closed account (S6k9GNh0)
Boost.Spirit gets good critique. Also, something new that's come out is Boost.xpressive which uses regex expressions (static or runtime expressions). Both give more than impressive results as far as optimization and usability goes.
Topic archived. No new replies allowed.