CODEWITHSUNDEEP
c++structunordered-map
Homberto
Homberto

Reputation: 165

C++ - unordered_map memory issues with struct

I decided to change my code today from vector to unordered_map so I can have string key values. However, it appears that unordered_map is not quite working out.

So basically I have a struct type:

typedef struct WindowData //the infamous Window Data struct
{
    HWND handle;
    std::unordered_map<std::string, WindowData*> children;
    COLORREF color;
    int height;
    int width;
    int x;
    int y;
    WindowData *parent;
    bool visible;
} windowData;

And then a globally-defined instance of it: 

WindowData MainWData = {NULL, std::unordered_map<std::string, WindowData*>(), NULL, 0, 0, 0, 0, NULL, true};

Then a function adds an element to the unordered_list (struct member children):

void GUI::CreateTitle(WindowData *data) //Home/About page title
{
    /*...*/
    WindowData wd={handle, std::unordered_map<std::string, WindowData*>(), ColorPalette.BackgroundColor, Nheight, Nwidth, x, y, data, true}; //all these values are defined within the scope of this function except ColorPalette, which is global
    data->children.insert(std::make_pair("title", &wd));
}

Finally, I have several other functions, including both members and non-members of the GUI class, that read the map element such as this one:

void GUI::CreateAboutButton(WindowData *data) //Home Page About Button
{
    /*...*/
    int y=data->children.at("title")->y + data->children.at("title")->height + 100;
    /*...*/
}

Now, to describe the error. Take int y from the GUI::CreateAboutButton() function. The value is supposed to be the same each time the program is run. Usually, it is something like 219. However, now, it changes every time the program is run. Sometimes y is the correct value. Other times it is 0. Other times it is greater that 40 000.

I know it is a memory issue, because sometimes when the program is run, it immediately signals a segfault, and when it doesn't, Dr. Memory shows two dozen "Uninitialized Read" errors. My guess is that because the unordered_map value has to be a pointer to a struct (the program wont compile if it is just the struct value and not a pointer), as soon as the struct instance wd from GUI::CreateTitle() goes out of scope, the map still points to its old memory location instead of the actual instance. But what I don't know how to do (this is my first time implementing an unordered_map) is fix the issue. I tried switching out the unordered_map::insert for an unordered_map::emplace, but this led to a segfault consistently.

Any help is appreciated.

EDIT: the diagnoses / solutions in the comments below lead me to solve the issue by defining wd as a public member of the class. It works fine now, and all memory errors are resolved.

Upvotes: 1

Views: 2126

Answers (1)

Juraj Blaho
Juraj Blaho

Reputation: 13451

The problem is in storing a pointer to a local variable wd in the map. The local variable is destroyed at the end of CreateTitle and the pointer in the map becomes dangling. One possible solution is for the map to own the children WindowData for example by using unique_ptr:

std::unordered_map<std::string, std::unique_ptr<WindowData>> children;

And then create it in place:

void GUI::CreateTitle(WindowData *data)
{
    /*...*/
    data->children.emplace(
        "title",
        make_unique<WindowData>(handle, std::unordered_map<std::string, std::unique_ptr<WindowData>>(), ColorPalette.BackgroundColor, Nheight, Nwidth, x, y, data, true)
    );
}

Add EDIT: Defining wd as a public member of WindowData only works if there is just one child. However then there would be no point in having the map, would it?

Upvotes: 3

Related Questions