Search code examples
c++pointersmemorymemory-leaksunions

Why does this union structure cause a memory leak?

I defined a union structure that can switch between a string pointer or a long (this is due to alignment issues). The structure itself looks as follows.

enum {H_STRING, H_LONG};
union Tag
{
    std::string *path;
    long id;
};

struct TextureID
{
    Tag tag;
    int type=H_STRING;

    TextureID()
    {
        type = H_STRING;
        tag.path = new std::string("");
    }
    TextureID(const TextureID& TID)
    {
        type = TID.type;
        if(type==H_STRING)
            tag.path = new std::string(*(TID.tag.path));
        else
            tag.id = TID.tag.id;
    }
    ~TextureID()
    {
        delete(tag.path);

    }

    TextureID& operator= (std::string str)
    {delete(tag.path); tag.path = new std::string(str); type=H_STRING; return *this;}
    TextureID& operator= (long val)
    { if(type==H_STRING) delete(tag.path); tag.id = val; type=H_LONG; return *this;}

    operator std::string&()
    {
        if(type == H_STRING)
        {
            return *(tag.path);
        }
    }
};

std::istream inline &operator>> (std::istream& is, TextureID& TID)
{is >> *(TID.tag.path); TID.type = H_STRING; return is;}

std::ostream inline &operator<< (std::ostream& os, TextureID& TID)
{return os << *(TID.tag.path);}

Using valgrind I have determined that this data structure, as is, has a memory leak.

The way to verify that this structure is the cause of the memory leak (i.e the reason why I am sure this is the cause and not something else) was to overload all operators currently being used (=, <<, >>) and have two versions of the data structure. the first is the one you see above using the union. the second simply has a string and a long as 2 separate fields in TextureID.

With the second implementation (the one not using pointers) there are no memory leaks.

I am aware this can cause a segmentation fault if the Tag is set to a long.That is not the issue, the issue is that somehow, despite there being an explicite call to delete() the allocated memory is not getting deleted (currently nothing in the program ever sets the tag value to a long so no seg fault occurs either).

EDIT:

It was requested that I provide proof of a memory leak so here it is:

enter image description here

This version does not cause a memory leak:

enum {H_STRING, H_LONG};

struct TextureID
{
    std::string path;
    long ID;
    int type=H_STRING;

    TextureID& operator= (std::string str)
    {path = str;}
    TextureID& operator= (long val)
    {ID = val;}

    operator std::string&()
    {
        if(type == H_STRING)
        {
            return (path);
        }
    }
};

std::istream inline &operator>> (std::istream& is, TextureID& TID)
{is >> TID.path; return is;}

std::ostream inline &operator<< (std::ostream& os, TextureID& TID)
{return os << TID.path;}
Solution

  • Your problem in your first code sample is almost certainly in

    TextureID& operator= (std::string str)
{delete(tag.path); tag.path = new std::string(str); type=H_STRING; return *this;}

    This ASSUMES that tag.path can be deleted. It will cause undefined behaviour if that isn't true - for example, if type == H_LONG.

    While it is debatable whether that genuinely causes a leak, symptoms of undefined behaviour can be anything, including spurious reports of a memory leak from tools like valgrind.

    In any event, a simple fix would be to change this operator to check if(type == H_STRING) before doing delete tag.path.

    The second example does not cause a leak, since the struct contains the members separately, and the compiler, by default, will ensure the destructor cleans all members up appropriately (invoking destructors, etc).

    As noted by PaulMcKenzie in comments, the second example has other issues that can also cause undefined behaviour (albeit, probably not a memory leak in practice). I'll leave those issues alone - that's going beyond the question asked.