iterator not working

[Athar]

TBH, I'm running out of ideas. But it seems likely that your list structure itself is damaged (e.g. a bad "next" pointer). Even valgrind can't detect all types of memory errors, mainly when still allocated memory is illegally accessed. When everything else fails, it usually leaves careful study of the source code (and many well-placed asserts).

The last few memory-related bugs I can remember were due to uninitialized members, so you might want to look out for that. I can't wait for GCC to support non-static member initialization, which will take care of the problem of uninitialized variables in C++ once and forever.

You also should consistently employ RAII (that means no deletes), which automatically takes care of the vast majority of memory-related problems.

[Caligulaminus]

Maybe you shouldn't update() via the const_iterator itr?

[xander333]

Why shouldn't I?

I was able to narrow it down to this function:

If I comment it out it executes perfectly fine.
Remark: bonuses isn't even mentioned in this function, yet when it is called it crashes on bonuses.

void Level::load(std::string fileName)
{
    amountOfBricks = 0;

    TiXmlDocument levelFile(fileName.c_str());
    if (!levelFile.LoadFile())
    {
        std::cout << "Loading level \"" << fileName << "\" failed." << std::endl;
        return;
    }

    TiXmlElement *map;
    map = levelFile.FirstChildElement("Level");

    TiXmlElement *layers;
    layers = map->FirstChildElement("Layers");

    TiXmlElement *layer;
    layer = layers->FirstChildElement("Layer");

    TiXmlElement *items;
    items = layer->FirstChildElement("Items");

    TiXmlElement *item;
    item = items->FirstChildElement("Item");

    while(item != NULL)
    {
        TiXmlElement *texture;
        texture = item->FirstChildElement("texture_filename");
        std::string texturename = texture->GetText();

        TiXmlElement *position;
        position = item->FirstChildElement("Position");

        TiXmlElement *X;
        X = position->FirstChildElement("X");
        int x = atoi(X->GetText());

        TiXmlElement *Y;
        Y = position->FirstChildElement("Y");
        int y = atoi(Y->GetText());

        //sprite->SetPosition(x - sprite->GetGlobalBounds().Width/2, y - sprite->GetGlobalBounds().Height/2);

        properties props;
        TiXmlElement *propElement;
        propElement = item->FirstChildElement("CustomProperties");

        TiXmlElement *prop;
        prop = propElement->FirstChildElement("Property");
        if(prop != NULL)
        {
            std::string propName = prop->Attribute("Name");
            if(propName == "stone")
                props.stone = true;
            else if(propName == "explosive")
                props.explosive = true;
        }

        Brick* temp = new Brick(texturename, props);
        bricks.push_back(temp);

        item = item->NextSiblingElement("Item");
        //delete sprite;
    }
    return;
}

[clanmjc]

Silly question, but is Bonus being modified in any other thread other than the thread that is executing the code in question. (Meaning, is it possible that bonus was modified, which would then invalidate the iterator to it... that said, the error would be something entirely different but I'm assuming this is multithreaded so I had to ask).

[xander333]

My application is single-threaded.
This is shaping up to become "that one crazy bug" i will still remember in 10 years lol.

[clanmjc]

I'm just going to throw some things out there. Is Update const qualified? const_iterator Behaves like const value_type*, it probably would give you a compile warning/error (Seriously doubt this has anything to do with it).

Straight out of Bjarne's book:

Independently of its category, an iterator can allow const or non-const access to the object it points to. You cannot write to an element using an iterator to const - whatever its category. An iterator provides a set of operators, but the type of the element pointed to is the final arbiter of what can be done to that element.
Read and writes copy objects, so element types must have the conventional copy semantics.

As a side note, is it detrimental to check if what the iterator points to is 0 and not call update if it is?

[xander333]

Update is indeed const qualified.
And I check if thr itr is NULL:

    for(std::list<Bonus*>::iterator itr = bonuses.begin(); itr != bonuses.end(); itr++)
    {
        if((*itr) != NULL)
            (*itr)->update();
    }

I also changed it to a normal iterator, just in case ;)

Now I get a seg fault on the if((*itr) != NULL) line.

[clanmjc]

Wait so the iter is null or what the iterator points to is null?

[mik2718]

Expect the iterator is not null, but the pointer Bonus* is

[xander333]

*itr means what it points to.
It crashes ON the condition, not what's inside the statement :s

[mik2718]

Sorry, you're right

must be the load function is corrupting memory

[xander333]

Indeed, but I just can't see where!

[clanmjc]

Let us know when you figure it out I'm very interested to see what it is.

[xander333]

Me too haha!
I tried running valgrind again but it gives no more information than we already have :(
I'm running out of ideas!

[xander333]

Static code analyzation shows nothing either.

[xander333]

I *may* have found something!
When debugging I kept an eye on the start and end address of the vector bricks.

First iteration:
_M_start = 0x462b318
_M_end = 0x462b33c

_M_end keeps incrementing ...
until suddenly!


Seventh iteration:
_M_start = 0x45419d0
_M_end = 0x4541a14

_M_end keeps once again incrementing...
until suddenly! (again)

Thirtheenth iteration:
_M_start = 0x36429e2
_M_end = 0x3642b24

Is this usual behaviour?

[clanmjc]

If the vector had to reallocate to make room for more elements I could see this happening. Does Bricks have anything to do with Bonus? Based on the iteration and frequency I'm thinking this is what is happen, now if it is related to the problem, not sure. Try specifying a capacity for the vector and running this test over, see if the same thing happens.

[xander333]

Will do!
I ran across another weird thingy in the mighty land of C++:
When I comment out the entire load function, this happens:

When I run the program with the debugger turned ON: It runs fine.
When I run the program with the debugger turned OFF (but still in debug mode compiled): It crashes.

[xander333]

Oh my oh my! very interesting development: using reserve() worked indeed and the _M_start position is no longer changed. However, when I go into the next function (update()) suddenly these vars show up:

_M_start = 0xfeeefeee
_M_end = 0xfeeefeee

how the hell did that happen?


EDIT: even better: every single one variable of my Level object is bullcrap!

[xander333]

I feel like crying. This is so definitely the most stupid bug ever.
At the start of the load function:
amountOfBricks = 0;

I forget to increment this variable each time I add a brick.
In the GameState class, which holds a vector of pairs, the next level is loaded when this variable goes down to 0.
Since I only added one level so far, it tried calling an object never existed. God fucking damn it, why didn't I look there earlier.

And thus the mysterious journey of this bug comes to an end.

I'd like to thank every single one of you for helping me trying to figure it out.

Thank you!