Question

I'm having trouble understanding why this isn't working as I expect it to. It may be that I'm using Visual Studio 2013, but hey.

This code is part of the item randomization system in a game engine I'm writing.

// the chance a rarity will have a given number of affixes
std::unordered_map<ItemRarities, ChanceSelector<int>> affixCountChances = {

std::pair<ItemRarities, ChanceSelector<int>>(ItemRarities::Cracked,
{ ChanceSelector<int>(
    { ChancePair(int, 100, 0) }) }),

std::pair<ItemRarities, ChanceSelector<int>>(ItemRarities::Normal,
{ ChanceSelector<int>(
    { ChancePair(int, 80, 0),
     ChancePair(int, 20, 1) }) }),

    // snip for conciseness (there are 3 more)
};

And this is the ChanceSelector class:

using Percentage = int;

#define ChancePair(T, p, v) std::pair<Percentage, T>(p, v)

template <class T>
class ChanceSelector
{
private:
    std::unordered_map<T, Percentage> _stuff;

public:
    ChanceSelector()
    {
    }

    ~ChanceSelector()
    {
        if (_stuff.size() > 0)
            _stuff.clear();
    }

    ChanceSelector(std::initializer_list<std::pair<Percentage, T>> list)
    {
        // snip for conciseness
    }

    T Choose()
    {
        // snip for conciseness
    }
};

The above code compiles fine but I have two questions:

  1. I don't understand why using ChanceSelector in std::pair requires a default constructor. Explicitly, it looks like I'm calling the constructor with the initializer list.
  2. When the applications runs, it crashes with: Unhandled exception at 0x01762fec in (my executable): 0xC0000005: Access violation reading location 0xfeeefeee.

Number 2 goes away if I only have one item in that map or if I change the definition of affixCountChances to std::unordered_map<ItemRarities, ChanceSelector<int>*> (and adjust the rest accordingly). The error dumps me at this code in list:

for (_Nodeptr _Pnext; _Pnode != this->_Myhead; _Pnode = _Pnext)
{
    _Pnext = this->_Nextnode(_Pnode); // <-- this line
    this->_Freenode(_Pnode);
}

Further inspection reveals the error happened in the destructor. _stuff is empty:

~ChanceSelector()
{
    if (_stuff.size() > 0)
    _stuff.clear();
}

It is legitly calling the destructor. Items are being removed from _stuff but I don't see why it would be calling the destructor. The crash happens after all items have been constructed and affixCountChances contains all items. I would assume that means it's destroying all the temporaries it created but I don't see why it would be creating temporaries.

Edit:

Constructor of ChanceSelector:

ChanceSelector(std::initializer_list<std::pair<Percentage, T>> list)
{
    int total = 0;
    int last = 100;
    for (auto& item : list)
    {
        last = item.first;
        total += item.first;
        _stuff[item.second] = total;
    }
    // total must equal 100 so that Choose always picks something
    assert(total == 100);
}
Was it helpful?

Solution

To answer your two questions:

  1. std::pair requires a default constructor, because you can do something like

    std::pair<int, MyClass> myPair();
    

    which creates a copy of your class using the default constructor (The values of the pair are actual values and not references):

    // MSVC implementation
    template<class _Ty1,class _Ty2>
    struct pair 
    {   // store a pair of values
    typedef pair<_Ty1, _Ty2> _Myt;
    typedef _Ty1 first_type;
    typedef _Ty2 second_type;
    
    pair()
    : first(), second() // Here your class gets default constructed
    {     // default construct
    }
    
    // .....
    _Ty1 first; // the first stored value
    _Ty2 second;    // the second stored value
    };
    

    The template of the pair gets fully implemented, so you need a default constructor event if you are not using the line above.

    One way to avoid this dependency is to use pointers in the std::pair, this then sets the default value of the second value of the pair to nullptr:

    std::pair<int, MyClass*> myPair();
    
  2. 0xFEEEFEEE indicates that the storage, where your pointer itself was stored has already been deleted (e.g. working on a deleted class reference). This deletion seems to occur somewhere outside the code you have posted here. For more Magic Numbers see Magic Numbers on Wikipedia

Edit:

Additionally, the contents of the initializer list do not exist after the constructor call. You might have there a reference copied instead of the actual object, which then gets deleted. The msvc implementation of the std::unordered_map uses a std::list as base for storing items. I'm not able to give your more information about this with the given code.

Initializer list and lifetime of its content

Edit 2: I was able to reproduce the error with your given code, it was not the content of the initializer_list ctor. The problem seems to be the lifetime of the objects inside the initializer list.

When I move the declaration of the pairs for the unordered map out of the initializer_list for the unordered map, everything works fine:

std::pair<ItemRarities, ChanceSelector<int>> pair1( ItemRarities::Cracked,
    { ChanceSelector<int>(
    { ChancePair( int, 100, 0 ) } ) } );
std::pair<ItemRarities, ChanceSelector<int>> pair2( ItemRarities::Normal,
    { ChanceSelector<int>(
    { ChancePair( int, 80, 0 ),
    ChancePair( int, 20, 1 ) } ) } );
std::unordered_map<ItemRarities, ChanceSelector<int>> chances = {
        pair1,
        pair2
    };

I'm not completely sure why this is a problem, but I think is comes from the {} in the initializer list, those objects might get deleted, when leaving the first {} and before entering the actual intializer_list for the unordered_map

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top