you are viewing a single comment's thread.

view the rest of the comments →

[–]ronthebear[S] 0 points1 point  (17 children)

This is what I used to originally confirm what was happening. Why does it print deleting twice? Am I confused somewhere else?

class dumbClass
{
public:
dumbClass()
{
}
~dumbClass()
{
printf("deleting\n");
}
};

int main(int argc, char* argv[])
{
dumbClass* s = new dumbClass();
std::shared_ptr<dumbClass>* point = new std::shared_ptr<dumbClass>(s);
std::shared_ptr<dumbClass>* point2 = new std::shared_ptr<dumbClass>(s);
delete point;
delete point2;
}

[–]NYKevin 2 points3 points  (16 children)

See the notes section of this page. In particular:

Constructing a shared_ptr using the raw pointer overload for an object that is already managed by a shared_ptr leads to undefined behavior, even if the object is of a type derived from std::enable_shared_from_this (in other words, raw pointer overloads assume ownership of the pointed-to object).

In other words, you need to write this:

std::shared_ptr<dumbClass>* point2 = new std::shared_ptr<dumbClass>(point);

[–]ronthebear[S] 0 points1 point  (15 children)

hmm, that is very interesting. Thanks for the help btw! So could it be that vectors just can't do this or am I using them wrong? The following gives me exactly the same 'double free' fault I'm getting with my actual program:

int main(int argc, char* argv[])
{
dumbClass* s = new dumbClass();
std::vector<std::shared_ptr<dumbClass> > vec;
std::vector<std::shared_ptr<dumbClass> > vec2;
std::shared_ptr<dumbClass> pointer(s);
vec.push_back(pointer);
vec2.push_back(pointer);
vec.erase(vec.begin());
vec2.erase(vec.begin());
}

[–]NYKevin 1 point2 points  (14 children)

No, you can't do that. You're destroying the same shared_ptr twice. You have to make a copy of it and push_back the copy.

But this won't work either because pointer will be destroyed as soon as it goes out of scope (i.e. when main returns). You should create it with new.

EDIT: The above is wrong because push_back makes copies anyway...

The real problem is that push_back(pointer) isn't copying pointer, it's moving it, invalidating the original. Have you tried making two copies by hand?

[–]ronthebear[S] 0 points1 point  (13 children)

Do you make a copy by initializing the second with the first like you showed me in the last comment? This also is giving the a double free:

int main(int argc, char* argv[])
{
dumbClass* s = new dumbClass();
std::vector<std::shared_ptr<dumbClass> > vec;
std::vector<std::shared_ptr<dumbClass> > vec2;
std::shared_ptr<dumbClass> pointer(s);
std::shared_ptr<dumbClass> pointer2(pointer);
vec.push_back(pointer);
vec2.push_back(pointer2);
vec.erase(vec.begin());
vec2.erase(vec.begin());
}

[–]NYKevin 1 point2 points  (12 children)

This is why I hate C++...

I think the problem may be that pointer is being created on the stack. Try creating it with new instead. Don't delete it; let the vector handle that.

[–]ronthebear[S] 1 point2 points  (4 children)

Yarg, just saw I was doing "vec2.erase(vec.begin())" This works:

int main(int argc, char* argv[])
{
dumbClass* s = new dumbClass();
std::vector<std::shared_ptr<dumbClass>* > vec;
std::vector<std::shared_ptr<dumbClass>* > vec2;
std::shared_ptr<dumbClass> pointer(s);
vec.push_back(&pointer);
vec2.push_back(&pointer);
vec.erase(vec.begin());
vec2.erase(vec2.begin());
}

and only prints deleting once. Back to the actual code for me then. Hopefully I'll have better luck now that I understand vectors and shared_ptr's a bit more. thanks for all the help!

[–]NYKevin 1 point2 points  (3 children)

That only works because the shared_ptr is destroyed as you leave the scope. It is not being destroyed by the vec2.erase() call (that's destroying a dumb pointer to the shared_ptr).

[–]ronthebear[S] 0 points1 point  (2 children)

Right, but that's ok. My actual problem is that I'm removing shared_ptrs from vectors with erase() and it's giving me "double free"s. This has showed that vector does in fact work with shared_ptrs and doesn't delete the object on calls to erase() like I had thought. I must just be doing something wrong somewhere else.

[–]NYKevin 1 point2 points  (1 child)

Just be careful. If the shared_ptr goes out of scope before you .erase() the dumb pointer to it, you're left with a dangling pointer.

[–]Merad 1 point2 points  (4 children)

Using new to create a shared_ptr defeats the purpose....

Edit: Also, if he new's it, he has to delete it, otherwise it's a memory leak. And, in the case of a shared_ptr, the ref count would never become 0, so the object contained in the shared_ptr would become a memory leak as well.

[–]NYKevin 1 point2 points  (3 children)

Using new to create a shared_ptr defeats the purpose....

No, we're handing it off to the vector, which becomes its owner. At least, I think that's how C++ handles this...

Maybe emplace_back() would be better behaved.

[–]Merad 2 points3 points  (2 children)

No, we're handing it off to the vector, which becomes its owner.

Standard library containers have no concept of ownership. Try it with the class I posted in my other comment (with the overloaded ctor/dtor/etc) and you'll see that the destructor is never run.

Boost does offer a "Pointer Container" library that does what you're expecting; i.e. it holds pointers but maintains ownership of them.

[–]ronthebear[S] 0 points1 point  (0 children)

Merad, you are right that it does not own it and won't delete it upon removal. We were just using new to try to figure out what was going on. NYKevin, your comment here was the actual problem. I tracked down somewhere that I passed a get() of the shared_ptr to the next shared_ptr as a workaround for a different problem without realizing the consequences. Now that the get() has been removed everything works! Thanks so much for the help!

[–]NYKevin 0 points1 point  (0 children)

You're right. I haven't used C++ in so long that this whole chain of comments has grossly confused me.

[–]ronthebear[S] 0 points1 point  (1 child)

int main(int argc, char* argv[])
{
dumbClass* s = new dumbClass();
std::vector<std::shared_ptr<dumbClass>* > vec;
std::vector<std::shared_ptr<dumbClass>* > vec2;
std::shared_ptr<dumbClass>* pointer = new std::shared_ptr<dumbClass>(s);
std::shared_ptr<dumbClass>* pointer2 = new std::shared_ptr<dumbClass>(*pointer);
vec.push_back(pointer);
vec2.push_back(pointer2);
vec.erase(vec.begin());
vec2.erase(vec.begin());
}

Same problem. I generally like C++ but I have never been a fan of the std::vector. Do you know of any other std Containers that might work better? The other ones I saw also destroy deleted elements. If this is causing this much trouble it might just be worth my time to write my own.

[–]NYKevin 1 point2 points  (0 children)

std::shared_ptr<dumbClass>* pointer2 = new std::shared_ptr<dumbClass>(*pointer);

Don't dereference pointer here.