Storing pointers in a std::vector causes invalid pointers?

[wh1t3crayon]

So I'm trying to store an object in one std::vector<Derived*>, and store a pointer to that object in another std::vector<Base*>. Accessing any element of the Base* vector results in an invalid pointer. I can solve this by making the first vector also a vector of pointers (std::vector<Derived*>). However, this requires me to instantiate every single Derived object with new and I want to avoid that. Here is the code showing what I am talking about:

// Project for SFML Web Problems.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"

class WorldObject{
public:
	WorldObject(){}
	virtual ~WorldObject(){}

	sf::Drawable &GetDrawable(){
		return *drawable;
	}
protected:
	sf::Drawable *drawable;
};

class Entity : public WorldObject{
public:
	Entity(){
		sprite.setPosition(20, 20);
		sprite.setFillColor(sf::Color::White);
		sprite.setSize(sf::Vector2f(50, 50));

		drawable = &sprite;
	}
	virtual ~Entity(){}

	sf::RectangleShape &GetSprite(){
		return sprite;
	}
private:
	sf::RectangleShape sprite;
};

class Holder{
public:
	Holder(){
//this is what I would like to do, but for some reason the pointer to entities[0] is invalid
		entities.push_back(Entity());
		objects.push_back(&entities[0]);	
//however, this works (assuming entities is std::vector<Entity*>):
        /*entities.push_back(new Entity());
		objects.push_back(entities[0]);
*/
	}
	~Holder(){}

	std::vector<WorldObject*> &GetObjects(){
		return objects;
	}
	std::vector<Entity> &GetEntities(){
		return entities;
	}
private:
	std::vector<WorldObject*> objects;
	std::vector<Entity> entities;
};

int _tmain(int argc, _TCHAR* argv[])
{
	sf::RenderWindow window(sf::VideoMode(500, 500, 32), "Test");
	sf::Event ev;

	Holder holder;

	while(window.isOpen()){
		while(window.pollEvent(ev)){
			if(ev.type == sf::Event::Closed){
				window.close();
			}
		}
		window.clear();
		//throws an invalid pointer error
		for(unsigned int i= 0; i < holder.GetObjects().size(); i++){
			window.draw(holder.GetObjects()[i]->GetDrawable());
		}
		window.display();
	}

	return 0;
}

So, avoiding the keyword new, how can I make these pointers valid?

[kbw]

So I'm trying to store an object in one std::vector<Derived>, ...

You mean std::vector<Derived*>, right?

[cire]

The problem is that if the vector of Derived objects grows, it may have to move the objects it contains which will invalidate pointers to those objects. One way around this is to use a container that doesn't have that issue, such as a std::deque.

[dhayden]

Another option: if you know that the second container points to objects in the first container, then make the second a container of indexes into the vector instead of a a container of pointers.

[wh1t3crayon]

So I'm trying to store an object in one std::vector<Derived>, ...

You mean std::vector<Derived*>, right?

Yep, nice catch.

The problem is that if the vector of Derived objects grows, it may have to move the objects it contains which will invalidate pointers to those objects. One way around this is to use a container that doesn't have that issue, such as a std::deque.

Ah, now I understand the issue. Thanks for the explanation.

Another option: if you know that the second container points to objects in the first container, then make the second a container of indexes into the vector instead of a a container of pointers.

That would work, but in the full scale problem, I have several std::vector<Base> containers, not just one.

Would a std::set<Base*> work just as well as std::deque

[wh1t3crayon]

Changing std::vector<Base*> to std::deque<Base*> did not work. The pointers in this container are still invalid.

[MiiNiPaa]

It is not you Base* container which gives you problems, it is Derived* one: you are storing ponters to Derived* container values in Base* one. As soon as you add another one entry to Derived* container, it can relocate, making all pointers to its elements invalid. You cannot avoid it by changing anything about Base* container.

It is like if you have a slip of paper with address, found out, that person lived here moved to another city and think that next time you prevent that by storing slip in your bag instead of your pocket.

You either need Derived* container to not relocate its content. Either use a container which does not reloate its content, like set, or make sure that container will not relocate (by reserving more space than you will ever need by vector). Alternatively, store pointer to vector itself and element index.

[Peter87]

entities.push_back(Entity());

Here you are creating a temporary object that will be copied to the vector. After the copy has been made the drawable pointer will no longer be valid.

What you could do instead is to use emplace_back to construct the object in-place inside the container:

entities.emplace_back();

http://en.cppreference.com/w/cpp/container/deque/emplace_back

[helios]

The one that should be a deque is Holder::entities.

[wh1t3crayon]

So I did some troubleshooting, and it would seem that the WorldObject pointers themselves are valid. The invalid pointer is sf::Drawable* inside of the WorldObject class. For some reason, this specific element is not being assigned correctly.

entities.push_back(Entity());


Here you are creating a temporary object that will be copied to the vector. After the copy has been made the drawable pointer will no longer be valid.

I tried using emplace_back, but that still resulted in the pointer to drawable being invalid. Am I giving the address for drawable incorrectly in my code?

Here is the runtime error:

Unhandled exception at 0x5E9CCC6E (sfml-graphics-d-2.dll) in Project for SFML Web Problems.exe: 0xC0000005: Access violation reading location 0xCCCCCCD0.

[wh1t3crayon]

So I got it to draw on screen by making sf::RectangleShape in my derived class a pointer as well. However, a runtime error is still thrown whenever I try to delete this pointer in the destructor.

// Project for SFML Web Problems.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"

class WorldObject{
public:
	WorldObject(){ drawable = nullptr; }
	virtual ~WorldObject(){}

	virtual sf::Drawable &GetDrawable(){
		return *drawable;
	}
protected:
	sf::Drawable *drawable;
};

class Entity : public WorldObject{
public:
	Entity(){
        //and I allocate the pointer with new
		sprite = new sf::RectangleShape();
		sprite->setPosition(20, 20);
		sprite->setFillColor(sf::Color::White);
		sprite->setSize(sf::Vector2f(50, 50));

		drawable = sprite;
	}
	virtual ~Entity(){
		//however, when I call:
                //delete sprite;
                //then another runtime error is thrown
	}

	virtual sf::Drawable &GetSprite(){
		return *sprite;
	}
private:
//so I made this a pointer now
	sf::RectangleShape *sprite;
};

class Holder{
public:
	Holder(){
		entities.push_back(Entity());
		objects.push_back(&entities[0]);	
	}
	~Holder(){}

	std::deque<WorldObject*> &GetObjects(){
		return objects;
	}
	std::deque<Entity> &GetEntities(){
		return entities;
	}
private:
	std::deque<WorldObject*> objects;
	std::deque<Entity> entities;
};

int _tmain(int argc, _TCHAR* argv[])
{
	sf::RenderWindow window(sf::VideoMode(500, 500, 32), "Test");
	sf::Event ev;

	Holder holder;

	while(window.isOpen()){
		while(window.pollEvent(ev)){
			if(ev.type == sf::Event::Closed){
				window.close();
			}
		}
		window.clear();
		for(unsigned int i= 0; i < holder.GetObjects().size(); i++){
			window.draw(holder.GetObjects()[i]->GetDrawable());
		}
		window.display();
	}

	return 0;
}

So I'm concerned about using new to allocate the above pointer. I know I need to have a delete for every new, but when I call delete then I get the following runtime error:

Unhandled exception at 0xCDCDCDCD in Project for SFML Web Problems.exe: 0xC0000005: Access violation executing location 0xCDCDCDCD.

Should I not be calling delete? Or is this not the correct fix for my problem in the first place?

[Peter87]

The object is being copied as I pointed out before but you have not implemented a copy constructor.

If you want it to be possible to copy the object you should implement a copy constructor (and copy assignment operator) to handle copying correctly.

Or you could decide to disable copying by declaring the copy constructor (and copy assignment operator) as deleted.

Entity(const Entity&) = delete;
Entity& operator=(const Entity&) = delete;

If the Entity was being copied you would then get an error at compile time instead of at runtime. This makes it much easier to spot where the copies are taking place.

I don't understand why emplace_back() didn't work. Are sure you called it without any arguments? I assumed you changed to a deque because emplace_back does not solve the problems pointed out by cire.

[wh1t3crayon]

I don't understand why emplace_back() didn't work. Are sure you called it without any arguments? I assumed you changed to a deque because emplace_back does not solve the problems pointed out by cire

Wow, now I feel stupid. I didn't know how emplace back worked. So now I can safely call delete sprite in Entity's destructor.

How would I implement emplace_back for std::deque<WorldObject*> objects;

In Holder(), I have

//works, because I am creating a new object. can call delete sprite;
entities.emplace_back();
//does not work, because I am trying to assign this address to a pointer. How do I
//correctly use emplace_back() with a reference to an object?
objects.emplace_back(&entities[0]);

Edit: Do I even need to delete drawable in the WorldObject destructor?

virtual ~WorldObject(){
//throws runtime error
		delete drawable;
	}

Edit edit:
I got the destructor to work as well, but only by setting drawable = nullptr in WorldObject() constructor. I've read that nullptr's are never a good sign. Is using nullptr okay here?