observation: maintaining self pointers in the boost way
A novice programmer once asked a master programmer, “Oh venerable master, can an instance of a class keep a shared_ptr to itself? If so will that class be ever deleted? Why would somebody want to do something like that?”
The master smiled and the following explanation happened.</b>
Experiment-1, dumb design and leaky code
Consider two classes Engine and Renderer.
Design-wise they can be related by composition.
Engine should have a Renderer.
Since we are speaking of shared_pointer-s, an instance of Engine should maintain
a shared_pointer to the instance of Renderer that it encompasses.
Also, the instance of Renderer needs to have a back-pointer to the engine that houses it,
because the Renderer need access to a number of characteristics of the engine
(viz: current camera, current scene, texture manager etc).
If the Renderer implements the back pointer via a shared_pointer to the Engine,
then we have a dead lock of destroying both the Engine and the Renderer. Engine has a shared_ptr to the Renderer
and Renderer has a shared_ptr to the Engine.
#include <cstdlib>
#include <crtdbg.h>
#include <string>
#include <boost/shared_ptr.hpp>
using namespace std;
using namespace boost;
namespace
{
namespace Experiment1
{
class R;
class E
{
public:
E( const std::string &data):m_data( data)
{
_RPT0(_CRT_WARN, "E:constructorn" );
}
virtual ~E()
{
_RPT0(_CRT_WARN, "E:destructorn" );
}
void SetRenderer(
shared_ptr <R> & renderer
)
{
m_renderer = renderer;
}
protected:
const std::string m_data;
shared_ptr <R> m_renderer;
};
class R
{
public:
R(
const std::string &data
):m_data( data)
{
_RPT0(_CRT_WARN, "R:constructorn" );
}
virtual ~R()
{
_RPT0(_CRT_WARN, "R:destructorn" );
}
void RegisterEngine(
shared_ptr <E> & engine
)
{
m_engine = engine;
}
protected:
const std::string m_data;
shared_ptr <E> m_engine;
};
}
If the above two classes are used in the following manner,
Engine has a shared_ptr to the Renderer and Renderer has a shared_ptr to the Engine.
Consequently the destructors of the instances pointed to by sR and sE wont be called,
which results in a memory leak.
#include <cstdlib>
#include <crtdbg.h>
#include <string>
#include <boost/shared_ptr.hpp>
/*
namespace Experiment1
{
// E and R classes defined as above
}
*/
using namespace Experiment1;
bool test()
{
//sR and sE refer to each other mutually
//consequently they are not deleted
//which results in memory leak
_CrtSetDbgFlag (
_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF
);
shared_ptr sE( new E("theEngine") );
shared_ptr sR( new R("cartoonRenderer"));
sE->SetRenderer( sR );
sR->RegisterEngine( sE );
return true;
}
Experiment2, leak proof code, but dumb design
To fix the memory leaks, R should hold a weak pointer to E.
#include <cstdlib>
#include <crtdbg.h>
#include <string>
#include <boost/shared_ptr.hpp>
#include <boost/weak_ptr.hpp>
using namespace std;
using namespace boost;
namespace
{
namespace Experiment2
{
class R;
class E
{
public:
E( const std::string &data):m_data( data)
{
_RPT0(_CRT_WARN, "E:constructorn" );
}
virtual ~E()
{
_RPT0(_CRT_WARN, "E:destructorn" );
}
void SetRenderer(
shared_ptr <R> & renderer
)
{
m_renderer = renderer;
}
protected:
const std::string m_data;
shared_ptr <R> m_renderer;
};
class R
{
public:
R(
const std::string &data
):m_data( data)
{
_RPT0(_CRT_WARN, "R:constructorn" );
}
virtual ~R()
{
_RPT0(_CRT_WARN, "R:destructorn" );
}
void RegisterEngine(
shared_ptr <E> & engine
)
{
m_engine = engine;
}
protected:
const std::string m_data;
weak_ptr <E> m_engine;
};
}
The following use of the Engine and Renderer wont cause any memory-leaks.
But it has a design problem as explained below.
#include <cstdlib>
#include <crtdbg.h>
#include <string>
#include <boost/shared_ptr.hpp>
/*
namespace Experiment2
{
// E and R classes defined as above
}
*/
using namespace Experiment2;
bool test()
{
//Memory leak free
//But not a good design.
_CrtSetDbgFlag (
_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF
);
shared_ptr sE( new E("theEngine") );
shared_ptr sR( new R("cartoonRenderer"));
sE->SetRenderer( sR );
sR->RegisterEngine( sE );
return true;
}
Experiment3, smart design, but leaky code
The requirement that the user has to explicitly have Renderer call
RegisterEngine after the Engine calls SetRenderer is called, is not a sound design.
Engine::SetRenderer should automatically call Renderer::RegisterEngine.
But inside the code of Engine::SetRenderer, for Engine to call Renderer::RegisterEngine,
how can the Engine pass in a shared_ptr or weak_ptr of itself to Renderer::RegisterEngine?
How about every instance of Engine, keeping a self shared_ptr of itself? This is a perfect example
of the necessity of keeping a self shared_ptr .
The novice programmer nodded in perfect agreement. But the master continued.
But the concept of shared_ptr is that each instance of a class
subjected to shared_ptr-ing should have only one shared reference structure.
If such a self shared_ptr is kept, then we should have any other shared_ptr of Engine
initialized by copy-constructing or assignment from the self shared_ptr
stored in Engine.
Otherwise we should have the same instance of Engine,
referenced by two or more shared_ptrs of Engine who don't share the same reference count structure.
Consequently we should 'private'-ise the constructor of Engine, and have Engine
created through a Create function.
This will again result in memory leak, since if you blindly keep a self shared_ptr to the class itself,
the class-s destructor will never be called. Consequently Engine's destructor will never be called.
#include <cstdlib>
#include <crtdbg.h>
#include <string>
#include <boost/shared_ptr.hpp>
#include <boost/weak_ptr.hpp>
using namespace std;
using namespace boost;
namespace
{
namespace Experiment3
{
class R;
class E
{
public:
shared_ptr &Create(
const std::string &i_data
)
{
E *pE = new E(i_data);
return pE->m_self;
}
virtual ~E()
{
_RPT0(_CRT_WARN, "E:destructorn" );
}
void SetRenderer(
shared_ptr <R> & renderer
);
protected:
E( const std::string &data):m_data( data)
{
m_self.reset( this );
_RPT0(_CRT_WARN, "E:constructorn" );
}
protected:
const std::string m_data;
shared_ptr <R> m_renderer;
shared_ptr <E> m_self;
};
class R
{
public:
R(
const std::string &data
):m_data( data)
{
_RPT0(_CRT_WARN, "R:constructorn" );
}
virtual ~R()
{
_RPT0(_CRT_WARN, "R:destructorn" );
}
void RegisterEngine(
shared_ptr <E> & engine
);
protected:
const std::string m_data;
weak_ptr <E> m_engine;
};
void E::SetRenderer(
shared_ptr <R> & renderer
)
{
m_renderer = renderer;
m_renderer->RegisterEngine( m_self );
}
void R::RegisterEngine(
shared_ptr <E> & engine
)
{
m_engine = engine;
}
}
The following use of te above design will still result in memory leak, since E is never deleted.
#include <cstdlib>
#include <crtdbg.h>
#include <string>
#include <boost/shared_ptr.hpp>
/*
namespace Experiment3
{
// E and R classes defined as above
}
*/
using namespace Experiment3;
bool test()
{
//A sound design but, instance pointed
//to by SE is never deleted. So memory leaks!
_CrtSetDbgFlag (
_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF
);
shared_ptr sE( E::Create("theEngine") );
shared_ptr sR( new R("cartoonRenderer"));
sE->SetRenderer( sR );
return true;
}
=== Experiment4, smart design and leak proof code
The correct way to do this is using a boost::enable_shared_from_this construct.
#include <cstdlib>
#include <crtdbg.h>
#include <string>
#include <boost/shared_ptr.hpp>
#include <boost/weak_ptr.hpp>
#include <boost/enable_shared_from_this.hpp>
using namespace std;
using namespace boost;
namespace
{
namespace Experiment4
{
class R;
class E :public enable_shared_from_this<E>
{
public:
shared_ptr &Create(
const std::string &i_data
)
{
shared_ptr temp( new E(i_data) );
return temp->shared_from_this();
}
virtual ~E()
{
_RPT0(_CRT_WARN, "E:destructorn" );
}
void SetRenderer(
shared_ptr <R> & renderer
);
protected:
E( const std::string &data):m_data( data)
{
_RPT0(_CRT_WARN, "E:constructorn" );
}
protected:
const std::string m_data;
shared_ptr <R> m_renderer;
//no need of maintaining a self shared_ptr
//shared_ptr <E> m_self;
};
class R
{
public:
R(
const std::string &data
):m_data( data)
{
_RPT0(_CRT_WARN, "R:constructorn" );
}
virtual ~R()
{
_RPT0(_CRT_WARN, "R:destructorn" );
}
void RegisterEngine(
shared_ptr <E> & engine
);
protected:
const std::string m_data;
weak_ptr <E> m_engine;
};
void E::SetRenderer(
shared_ptr <R> & renderer
)
{
m_renderer = renderer;
m_renderer->RegisterEngine( m_self );
}
void R::RegisterEngine(
shared_ptr <E> & engine
)
{
m_engine = engine;
}
}
This is used as below.
#include <cstdlib>
#include <crtdbg.h>
#include <string>
#include <boost/shared_ptr.hpp>
/*
namespace Experiment4
{
// E and R classes defined as above
}
*/
using namespace Experiment4;
bool test()
{
//The correct design which doesnt result in memory leaks.
_CrtSetDbgFlag (
_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF
);
shared_ptr sE( E::Create("theEngine") );
shared_ptr sR( new R("cartoonRenderer"));
sE->SetRenderer( sR );
return true;
}
blog comments powered by Disqus