Help understanding how shared_ptr's work?

[wh1t3crayon]

In another post I was workig through using std::shared_ptr's in my program, but since this is specifically about how they work I figured I'd start a new thread. Anyways, my question is that I thought that altering a member of a shared_ptr would also alter the same member of the instance that that object points to, but in this code I made to demonstrate the problem, the object's member isn't altered when the shared_ptr's member is, but if I use regular pointers then changing the pointer's member also changes the object's member. Okay, here is the code

#include "stdafx.h"
#include <memory>
#include <iostream>

class Object{
public:
	Object(){i = 0;} //so initially, i == 0
	~Object(){}

	int i; //the object member I am referring to
};

int _tmain(int argc, _TCHAR* argv[]){
	std::vector<Object> instances; //using vectors for consistancy because my actual program uses them
	Object object; //the actual object instance, i == 0
	instances.push_back(object);

	std::vector<Object*> regularPointers; //a regular pointer, changing the pointer's i member also changes the object's i
	regularPointers.push_back(&instances[0]);

	std::vector<std::shared_ptr<Object>> sharedPointers;
	sharedPointers.push_back(std::make_shared<Object>(instances[0])); //push back a shared pointer to the object instance, 
//this pointer seems to have no bearing on the actual object instance however
	
	sharedPointers[0]->i = 10;
	std::cout << instances[0].i << " "; //altering a shared pointer DOES NOT alter the object instance, i == 0

	regularPointers[0]->i = 10;
	std::cout << instances[0].i << " "; //altering a regular pointer DOES alter the object instance, i == 10

	instances[0].i = 10;
	std::cout << instances[0].i << " "; //proves that the object instance was altered, i == 10 instead of 0
	system("pause");

	return 0;
}

So, my understanding of shared ptr's is that they act just like a dumb pointer, except with automatic deallocation and the ability to have multiples of them. However, I can't seem to actual alter the object that the shared pointer points to, even though I can alter the same object via a dumb pointer. The output should have been "10 10 10" but it was "0 10 10" because using sharedPointers[0]-> didn't actually change anything of objects[0]

[MiiNiPaa]

make_shared creates a copy of passed object.
It is important that shared_ptr should not point to any object which is already managed by another object.
You can do sharedPointers.emplace_back(&instances[0]), but this would be wrong. It will lead to the double delete problem as now there two owners of single object: original vector and shared_pointer.
All objects should have only one owner.

[tipaye]

in line 22, make_shared allocates and initializes an object in dynamic memory
and returns a shared_ptr that points to that object.

I think you were expecting make_shared to just somehow return a shared_ptr pointing to the existing object

[MikeyBoy]

The whole point of a shared pointer is that it deletes the object that it's pointing to, once its reference count drops to zero. What do you think will happen when it tries to delete an object that you haven't dynamically allocated in the first place?

Ignore that - brain not working right this afternoon.

[wh1t3crayon]

I think you were expecting make_shared to just somehow return a shared_ptr pointing to the existing object

I was expecting exactly that. If I have a class that holds a vector of objects instances, and another class that needs access to those specific instances, how do I achieve that if I can't use a shared pointer? Like this

class Holder{
public:
	Holder();
	~Holder();

	std::vector<Object> instances;
};

class PointerHolder{
public:
	PointerHolder();
	~PointerHolder();

	/*I want this class to hold a vector of the same instances
        of the objects that are in the Holder class*/
}

If PointerHolder can't access the object instances with shared pointers, what other options do I have (besides dumb pointers)?

Oh I just thought of an idea, if make_shared<>() basically makes a new object, then could I just turn all of my instances into a shared_ptr? I.e. change std::vector<Object> instances; to std::vector<std::shared_ptr<Object>> instances; or would that not work because now my objects would have no real instance of themselves?
That didn't seem to work, I kept getting error C2664 because this line holder.instances.push_back(std::make_shared<Object>(object)); gave me this error

error C2664: 'Object::Object(const Object &)' : cannot convert parameter 1 from 'std::shared_ptr<_Ty>' to 'const Object &' c:\program files (x86)\microsoft visual studio 11.0\vc\include\memory 873

[keskiverto]

http://www.cplusplus.com/reference/memory/shared_ptr/operator=/
http://www.cplusplus.com/reference/memory/shared_ptr/shared_ptr/

#include <vector>
#include <memory>
#include <iostream>

struct Foo {
    int x;
    Foo(int y) : x(y) {}
    ~Foo() {
        std::cout << "~Foo; x==" << x << '\n';
    }
};

int main() {
	std::vector<std::shared_ptr<Foo>> owners;
	owners.push_back( std::make_shared<Foo>(42) ); // #1

	std::vector<std::shared_ptr<Foo>> others;
	others.push_back( owners[0] ); // #2
	std::cout << owners[0]->x << " " << others[0]->x << '\n';
	others[0]->x = 7;
	std::cout << owners[0]->x << " " << others[0]->x << '\n';
	std::cout << "Prepare" << std::endl;
	owners.clear(); // #3
	std::cout << "to die" << std::endl;

	return 0; // #4
}

#1 One Foo is created dynamically. A pointer to it is stored in owners.

#2 The pointer is copied from owners to others. Both point to the one and only Foo.

#3 The first pointer is destroyed. The others still has a pointer to the Foo object, so the object lives on.

#4 The others leaves scope. The pointer that it has is destroyed. The pointer deletes the Foo.

[wh1t3crayon]

Thanks, that's a great example. One problem though, at #1 where your code isowners.push_back( std::make_shared<Foo>(42) ); // #1 I have holder.instances.push_back(std::make_shared<Object>(3)); and I get error C2664, which is what I listed above. Why is that? What's different?

Here are my classes for reference

public:
	Object(int x){i = x;}
	~Object(){}

	int i;
};

class Holder{
public:
	Holder(){}
	~Holder(){}

	std::vector<std::shared_ptr<Object>> instances;
};

[MikeyBoy]

I was expecting exactly that.

The whole point of a shared pointer is that it deletes the object that it's pointing to, once its reference count drops to zero. What do you think will happen when it tries to delete an object that you haven't dynamically allocated in the first place?

Having a shared pointer to memory you didn't allocate would be pointless and meaningless at best - and, actually, is downright dangerous. What benefit did you think you were going to get from doing it?

[S G H]

Mikey, take a little break? ;p
@OP: looks good to me. Try cleaning up the project and recompiling it.
The error should happen if you had a std::vector<Object> instead, if this can help you.

[wh1t3crayon]

@MikeyBoy so I should have a vector of instances along with a vector of shared pointers pointing to those instances? What about keskiverto's code example? He never has an object instance, he just allocates new shared pointers in vectors. That's what I meant when I said I had an idea. As for my error, I'm on my phone, but I'll try cleaning and rebuilding tomorrow morning.

[keskiverto]

wh1t3crayon wrote:

@MikeyBoy so I should have a vector of instances along with a vector of shared pointers pointing to those instances?

No. That is not what Mikey said.

MikeyBoy wrote:

Having a shared pointer to memory you didn't allocate would be pointless and meaningless at best - and, actually, is downright dangerous.

He says that shared pointer to instance in vector is very bad.

Even simpler example of that:

{
  int foo = 42;
  int * bar = &foo;
  delete bar; // error
}

It should be clear why that is wrong. This is exactly the same, with the delete hidden in ~shared_ptr():

{
  int foo = 42;
  shared_ptr<int> bar( &foo );
  // error
}

What does the vector do?

{
  std::vector<int> vfoo( 1, 42 );
}
// that is equivalent to:
{
  int * pfoo = new int [1];
  pfoo[0] = 42;

  delete [] pfoo;
}

Yes, the vector allocates dynamically, but it also "owns" what it allocates. The vector does not know anything about shared pointers. Even vector's own iterators invalidate on some vector operations. When you give a pointer to shared_ptr, it assumes that it owns the object alone. When you copy a shared_ptr, both copies know of each other and have joint custody of the object.

The make_shared is just syntactic sugar that simplifies code:

typedef std::shared_ptr<Object> SOB;

SOB foo = std::make_shared<Object> (10);
// same as:
SOB foo2( new Object(10) );
// same as:
Object * temp = new Object(10);
SOB foo3( temp );

As for the compiler error, a clean build is indeed the first thing to try. The next is to recheck every type in the code. (The typedefs are nice for "set in one place rather than search-and-replace all over".) Last resurt is to suspect that a newer compiler could be more "understanding".

[tipaye]

As per your compiler error, have you solved that?

I expected that error when you had:

std::vector<Object> instances;
...
instances.push_back(std::make_shared<Object>(object));

Because instances holds Objects and you're trying to store shared_ptr<Object> in it.

But after you changed to:
std::vector<std::shared_ptr<Object>> instances;
I expect things to work.

[wh1t3crayon]

Yeah that's what I changed it to. Cleaning and rebuilding didn't work, so in case my eyes aren't working I'll just post everything I have in this little test:

#include "stdafx.h"
#include <memory>
#include <iostream>

class Object{
public:
	Object(){}
	~Object(){}

	int i;
};

class Holder{
public:
	Holder(){}
	~Holder(){}

	std::vector<std::shared_ptr<Object>> instances; //so, a vector of shared pointers...
};

class PointerHolder{
public:
	PointerHolder(){}
	~PointerHolder(){}

	std::vector<std::shared_ptr<Object>> sharedPointers;
};

int _tmain(int argc, _TCHAR* argv[]){
	Holder holder;
	holder.instances.push_back(std::make_shared<Object>()); //allocating a new Object, throws the error

	PointerHolder ph;
	ph.sharedPointers.push_back(std::make_shared<Object>(holder.instances[0]));
	
	ph.sharedPointers[0]->i = 10;
	std::cout << holder.instances[0]->i << " "; //altering a shared pointer DOES NOT alter the object instance, i == 0

	holder.instances[0]->i = 10;
	std::cout << holder.instances[0]->i << " "; //proves that the object instance was altered, i == 10 instead of 0
	system("pause");

	return 0;
}

[MikeyBoy]

I'm not seeing a problem with like 31. std::make_shared<Object>() should return a shared pointer to an Object, which is exactly what you're storing in holder.

Line 34 looks fishy to me. Firstly, using std::make_shared<Object> creates a new instance of Object. Is that really what you're intending to do?

Secondly, holder.instances[0] is a shared pointer. The arguments you pass to std::make_shared are the arguments that get passed through to the constructor of the thing you're trying to create. You're effectively trying to invoke a constructor Object(std::shared_ptr<Object>) - which doesn't exist!

Finally, I can only second keskiverto's suggestion that you use a typedef for std::shared_ptr<Object>. It would make your code clearer, so that you can see more easily what you're doing, and where you might be going wrong.

[tipaye]

ph.sharedPointers.push_back(std::make_shared<Object>(holder.instances[0]));

You're trying to invoke the Object copy constructor, but instead of passing a const Object &, you're passing a shared_ptr<Object>.

You need to de-reference the shared_ptr like so:
ph.sharedPointers.push_back(std::make_shared<Object>(*holder.instances[0]));

Edit:
Must have been dozing as well, you shouldn't be doing make_shared again, as explained below. Apologies and thanks.

[keskiverto]

You need to de-reference the shared_ptr like

No!

If the purpose of the pointers in ph is to point to the same objects that the pointers in holder, then you absolutely do not make_shared.

Line 34 should be:
ph.sharedPointers.push_back( holder.instances[0] );
(That was the only code change to make it compile, line 31 was ok.)

If you would make_shared, then the new shared_ptr in ph would not know about the shared_ptr in holder, even they would point to the same Object object. That is not sharing and you will experience a double delete crash.


PS. Do the <memory> and <iostream> include <vector> in VS2012?

[wh1t3crayon]

Wow, my eyes were indeed not working. Line 34 was the issue, I was making shared an already shared pointer instance... Thanks for that help. Anyways, I'll apply this newly working application to my large scope project and see if things will now work as intended.

PS. Do the <memory> and <iostream> include <vector> in VS2012?

Well, no, but strangely enough I've never had an issue without including <vector>.

[wh1t3crayon]

Okay, I'm stuck integrating what I have learned. When passing an already made shared object through a function to add it to a vector in a seperate class, I get this error:

error C2664: 'GridSquare::AddStationaryObject' : cannot convert parameter 1 from 'std::shared_ptr<_Ty>' to 'std::shared_ptr<_Ty> &' c:\users\jordan\documents\visual studio 2012\projects\collision grid test\collision grid test\board.cpp 55

Here is the code causing the error:

//I know there are a lot of constants and undefined var's here, but the issue
//only involves the creation of my Wall objects

//10 * 10 grid, so each sq. is 1/10 the width and height
	int xcounter = 0, ycounter = 0;
	for(int i = 0; i < (SCREEN_WIDTH / GRIDSQUARE_WIDTH) * (SCREEN_HEIGHT / GRIDSQUARE_WIDTH); i++){
		grid_.push_back(GridSquare(xcounter * GRIDSQUARE_WIDTH, ycounter * GRIDSQUARE_HEIGHT, GRIDSQUARE_WIDTH, GRIDSQUARE_HEIGHT));
		//1 wall per gridsquare
		for(int j = 0; j < 1; j++){
//creates a wall as a shared_ptr
			Wall wall(xcounter * GRIDSQUARE_WIDTH, ycounter * GRIDSQUARE_HEIGHT);
			walls_.push_back(std::make_shared<Wall>(wall));
		}

		xcounter++;
		if(xcounter % 10 == 0){
			xcounter = 0;
			ycounter++;
		}
	}
//...

for(int i = 0; i < grid_.size(); i++){
		//for every wall as created above
		for(int j = 0; j < walls_.size(); j++){
			sf::FloatRect wallfr(walls_[j]->GetActiveSprite().getGlobalBounds());
			if(grid_[i].GetGridSquare().intersects(wallfr)){
//these lines throw the error
				grid_[i].AddStationaryObject(walls_[j]);
				grid_[i].AddGridObject(walls_[j]);
			}
		}

And here are the functions, located in the class GridSquare

void AddStationaryObject(std::shared_ptr<WorldObject> &object);
	void AddGridObject(std::shared_ptr<WorldObject> &object);
//...
void GridSquare::AddStationaryObject(std::shared_ptr<WorldObject> &object){
	stationaryObjects_.push_back(object);
}
void GridSquare::AddGridObject(std::shared_ptr<WorldObject> &object){
	gridObjects_.push_back(object);
}

[MikeyBoy]

What's the definition of walls_ ?

What's the relationship between Wall and WorldObject? Does the former derive from the latter?

[wh1t3crayon]

Right, here is walls_std::vector<std::shared_ptr<Wall>> walls_; and here is Wall deriving from WorldObject

class Wall : public WorldObject{
//...
};

I'll also add that changing the functions to

void GridSquare::AddStationaryObject(std::shared_ptr<WorldObject> object){
	stationaryObjects_.push_back(object);
}
void GridSquare::AddGridObject(std::shared_ptr<WorldObject> object){
	gridObjects_.push_back(object);
}

I.e. passing by value instead of reference, gets rid of the errors, but I'm pretty sure this would cause my one shared_ptr to be split into two different shared_ptr's which wouldn't work.

It would also appear that inheritance simply isn't working as I expect it to here. For instance, passing an object of equal inheritance level to the above functions, (where they take it by ref. not value) works just fine, but if the object being passed is further down the inheritance chain, then the C2664 error is thrown. Here is an example

class WorldObject{
//...
};

class MoveableObject : public WorldObject{
//...
};

class Ball : public MoveableObject{
//...
};

std::vector<std::shared_ptr<WorldObject>> objects;
std::vector<std::shared_ptr<MoveableObject>> moveableObjects;
Ball ball;
objects.push_back(std::make_shared<WorldObject>(ball));
moveableObjects.push_back(objects[0]);
gridsquare::AddWorldObject(moveableObjects[0]); //doesn't work because moveableObjects isn't a world object, it only derives from it. but...
GridSquare::AddWorldObject(objects[0]); //DOES work since objects[0] is a world object, although object[0] is simply a Ball... 

So in short, I'm pretty upset about this pickiness. Are there any solutions to this, or have I done something wrong?

Edit Number 4: Wow, all that typing and all I had to do was simply type 'const' in the function parameters... Well, at least I learned something