Reputation: 4174
Can someone tell me the difference between these two methods when used as below?
If I use CreateBoostContainer, my code works well. If I use CreateContainer however, I get a boost::bad_weak_ptr exception later in the code in function Foo when trying to use shared_from_this on the ContainerC. I'm using only one thread.
Thanks!
Usage:
SceneElementNodeC* poNode(new SceneElementNodeC(CreateBoostContainer()));
SceneElementNodeC* poNode(new SceneElementNodeC(boost::shared_ptr<SceneElementI>(CreateContainer())));
Definitions:
boost::shared_ptr<SceneElementI> LoaderC::CreateBoostContainer() const
{
return boost::shared_ptr<SceneElementI>(new ContainerC());
}
SceneElementI* LoaderC::CreateContainer() const
{
return new ContainerC();
}
SceneElementNodeC:
class SceneElementNodeC
{
SceneElementNodeC(boost::shared_ptr<SceneElementI> spSceneElement)
: m_spSceneElement(spSceneElement)
{};
}
ContainerC:
class ContainerC : public SceneElementI, public boost::enable_shared_from_this<ContainerC>
{
ContainerC()
{};
void Foo(VisitorI* poVisitor)
{
poVisitor->Visit(shared_from_this());
};
}
Upvotes: 0
Views: 151
Reputation: 171373
Firstly, CreateBoostContainer
is a terrible name, the Boost library contains hundreds of components including several containers, shared_ptr
is just one small part of Boost. If you later changed the code to return a std::shared_ptr
instead you'd have to rename the functions, so would you change it to CreateStdContainer
?!
Secondly you have failed to provide complete code that allows the problem to be reproduced, so according to the rules of Stackoverflow your question should be closed! I am guessing that your type is defined like this:
class ContainerC
: public SceneElementI, public boost::enable_shared_from_this<ContainerC>
{
// ...
};
The difference is that in this function:
boost::shared_ptr<SceneElementI> LoaderC::CreateBoostContainer() const
{
return boost::shared_ptr<SceneElementI>(new ContainerC());
}
You initialize the shared_ptr
with a ContainerC*
so the shared_ptr
constructor is able to detect that there is a enable_shared_from_this
base class (by an implicit upcast to enable_shared_from_this<ContainerC>
)
Whereas in this function:
SceneElementI* LoaderC::CreateContainer() const
{
return new ContainerC();
}
you lose information about the dynamic type of the object by converting the pointer to the base class before creating the shared_ptr
. The returned pointer is SceneElementI*
which (I assume) does not have an enable_shared_from_this
base class, so when that pointer is later used to initialize a shared_ptr
it is not possible to tell that it points to a base class of a type that is also derived from enable_shared_from_this<ContainerC>
.
You could make the first function also fail to work by doing this:
boost::shared_ptr<SceneElementI> LoaderC::CreateBoostContainer() const
{
SceneElementI* ptr = new ContainerC();
return boost::shared_ptr<SceneElementI>(ptr);
}
This is equivalent, because it converts the pointer to SceneElement*
before creating the shared_ptr
.
shared_ptr
is clever and makes use of the pointer type you construct it with even if that is not the type being stored in the shared_ptr
, so if you upcast that pointer first then shared_ptr
is not able to be clever.
Upvotes: 1
Reputation: 249
Using the raw pointer at all is poor form. This is why it's typical to use something like std::make_shared
- to prevent any possible memory leak. Consider that an exception may be thrown between the point at which you new the class and when it gets associated to a smart pointer.
Having said that, I believe this is just an implementation detail of how boost's enable_shared_from_this works.
Upvotes: 1
Reputation: 51263
Documentation for enable_shared_from_this
Requires: enable_shared_from_this must be an accessible base class of T. *this must be a subobject of an instance t of type T . There must exist at least one shared_ptr instance p that owns t.
Note, "There must exist at least one shared_ptr instance p that owns t".
If you use CreateContainer
then there is no shared_ptr
instance that owns it.
Upvotes: 1