Sign in to follow this  
bkt

shared_ptr usage

Recommended Posts

I've never used boost::shared_ptr before, but I think that I'm getting problems at compile time with the boost::shared_ptr library. Here's the errors I get, and the lines that cooresponds with it.
c:\Documents and Settings\Johnny\Desktop\engine\core\engine.cpp(31): error C2440: 'delete' : cannot convert from 'boost::shared_ptr<T>' to 'void *'
        with
        [
            T=logFile
        ]
c:\Documents and Settings\Johnny\Desktop\engine\core\engine.cpp(27): error C2440: 'delete' : cannot convert from 'boost::shared_ptr<T>' to 'void *'
        with
        [
            T=CKernel
        ]
c:\Documents and Settings\Johnny\Desktop\engine\core\engine.cpp(15): error C2679: binary '=' : no operator found which takes a right-hand operand of type 'logFile *' (or there is no acceptable conversion)
c:\Documents and Settings\Johnny\Desktop\engine\core\engine.cpp(14): error C2679: binary '=' : no operator found which takes a right-hand operand of type 'CKernel *' (or there is no acceptable conversion)

Here are some of the lines:
	m_pKernel = new CKernel;
	log		  = new logFile("log.txt");
---
	delete m_pKernel;
//	CEngineVar::release();

	log->close();
	delete log;

Share this post


Link to post
Share on other sites
Well if m_pKernel is a shared_ptr you wouldn't delete it, you let it go out of scope to delete it.you can also call m_pKernel.Release() on it.

Cheers
Chris

Share this post


Link to post
Share on other sites
boost::shared_ptr has weird symantics. You need to use the constructor to initialize it:


// assuming m_pKernel and log are boost::shared_ptrs
m_pKernel = boost::shared_ptr<CKernel>(new CKernel);
log = boost::shared_ptr<logFile>(new logFile("log.txt"));


And like chollida1 said, you shouldn't call delete on smart pointers. shared_ptr will destroy itself automatically when it's ref count reaches 0.

Share this post


Link to post
Share on other sites

boost::shared_ptr<logFile> log( new logFile("log.txt") );

when the shared_ptr (log) goes out of scope, the logFile instance is automatically deleted for you. keep in mind that shared_ptr is also reference counted and can be stored in stl containers(unlike auto_ptr).

Share this post


Link to post
Share on other sites
I have a linked list of shared_ptr<ITask> and I'm looking to typecast down from a shared_ptr<VideoTask>, shared_ptr<TimerTask>, shared_ptr<LogicTask> into the list (they all devrive from the ITask object). Here's the code I've been trying to get working:

taskList.push_back( shared_ptr<ITask> (reinterpret_cast<ITask> (&m_pTimer)) );
taskList.push_back( shared_ptr<ITask> (reinterpret_cast<ITask> (&m_pVideo)) );
taskList.push_back( shared_ptr<ITask> (reinterpret_cast<ITask> (&m_pLogic)) );

Here's the linked list object:

class CTaskList : public list< shared_ptr<ITask> >
{
};
But that doesn't seem to be liking me too much. I'll fiddle around with a little bit more, but from what I'm gathering, the ampersand operator doesn't return a pointer to the object inside the shared_ptr?

Share this post


Link to post
Share on other sites
Quote:
Original post by bkt
But that doesn't seem to be liking me too much. I'll fiddle around with a little bit more, but from what I'm gathering, the ampersand operator doesn't return a pointer to the object inside the shared_ptr?


Nope, it's getting the addrses of the shared_ptr structure itself (similarly, using the ampersand operator on a pointer would get a pointer to a pointer, not return the original pointer). Use the .get() member to get a raw pointer. Note: Constructing a second shared_ptr from that raw pointer will in fact cause the item to be deleted twice!!! So don't do this:
boost::shared_ptr< foo > foo1( new foo );
boost::shared_ptr< foo > foo2( foo1.get() ); //ERROR: We'll delete foo twice!!!

Or do so indirectly using casts.
I'm curious as to the types of m_pTimer, m_pVideo, and m_pLogic are. If they are of types that derive from ITask, then there's no need to do any casting jumbo:
struct ITask {};
struct TimerTask : public ITask {};
boost::shared_ptr< TimerTask > m_pTimer( new TimerTask );
boost::shared_ptr< ITask > foo( m_pTimer );

For 3 of the 4 _cast<> operators, there is a boost equivilant. Let's assume we want to use dynamic_cast to ensure a pointer to an ITask is in fact a TimerTask. We could write:
boost::shared_ptr< ITask > foo( ... );
boost::shared_ptr< TimerTask > Timer( dynamic_cast< TimerTask * >( foo.get() ) );

This would be bad, as we would have constructed Timer from a raw pointer (causing the double delete problem originally outlined). The alternative:
boost::shared_ptr< ITask > foo( ... );
boost::shared_ptr< TimerTask > Timer( boost::dynamic_pointer_cast< TimerTask >( foo ) );

This code will not delete our task twice, and will in fact function as expected.

The full list of casts available are:
static_cast< T * >  -> boost::static_pointer_cast< T >
const_cast< T * > -> boost::const_pointer_cast< T >
dynamic_cast< T * > -> boost::dynamic_pointer_cast< T >

More Documentation

Share this post


Link to post
Share on other sites
No, actually, m_pVideo, m_pLogic, and m_pTimer are setup just as you've shown here:

struct ITask {};

struct TimerTask : public ITask {};

boost::shared_ptr< TimerTask > m_pTimer( new TimerTask );

boost::shared_ptr< ITask > foo( m_pTimer );
But the problem is that when I do that (because it does compile fine, that was the first thing that I tried) the code crashed and gave me a NULL pointer assertion when I attempted to use the shared_ptr from the list< shared_ptr<ITask> >.

Share this post


Link to post
Share on other sites
Quote:
Original post by bkt
But the problem is that when I do that (because it does compile fine, that was the first thing that I tried) the code crashed and gave me a NULL pointer assertion when I attempted to use the shared_ptr from the list< shared_ptr<ITask> >.


This is odd, because this code is perfect. Your problem will be elsewhere then.

struct ITask {};
struct TimerTask : public ITask {};
boost::shared_ptr<TimerTask> m_pTimer(new TimerTask);
boost::shared_ptr<ITask> foo(m_pTimer);


Your problem probably has something to do with:


taskList.push_back( shared_ptr<ITask> (reinterpret_cast<ITask> (&m_pTimer)) );
taskList.push_back( shared_ptr<ITask> (reinterpret_cast<ITask> (&m_pVideo)) );
taskList.push_back( shared_ptr<ITask> (reinterpret_cast<ITask> (&m_pLogic)) );


Which it utterly terrible. Try this code:
// definitions
std::list<shared_ptr<ITask> > tasklist;
shared_ptr<TimerTask> m_pTimer(new TimerTask);
// etc...

// the pushing
taskList.push_back(shared_ptr<ITask>(m_pTimer));
taskList.push_back(shared_ptr<ITask>(m_pVideo));
taskList.push_back(shared_ptr<ITask>(m_pLogic));


You probably want to read the manual on reinterpret_cast. It has absolutly no business being used in that code.

Share this post


Link to post
Share on other sites
That's what I changed it to afterwords when it didn't work, before hand though, this is what it looked like:

taskList.push_back( m_pTimer );
taskList.push_back( m_pVideo );
taskList.push_back( m_pLogic );

I only started tampering with it when that gave me the assert fail.

Share this post


Link to post
Share on other sites
I've had a chance to run everything through the debugger, and for some reason the pointers inside the list seem to be NULL after I pass them on to the boot function of my kernel object. Here's the code:

void CKernel::boot(CTaskList taskList)
{

m_TaskList = taskList;

for(CTaskList::iterator it=m_TaskList.begin();
it != m_TaskList.end(); it++)
LINK_TASK_TO_KERNEL( (*it) );

run();
}

CEngine::CEngine(void)
{
m_pKernel = shared_ptr<CKernel> (new CKernel);
log = shared_ptr<logFile> (new logFile("log.txt"));

CTaskList taskList; // used to pass into the kernel booting process

// we need to add the tasks to the task list in order for the kernel to run
// them. this makes everything as abstract as possible so we can cut and paste
taskList.push_back( m_pTimer );
taskList.push_back( m_pVideo );
taskList.push_back( m_pLogic );

m_pKernel->boot( taskList );
// *** we're running at this point, and should only exit afrer the loop dies ***
}


I'm sorry about all these questions, I've just never used these shared pointers before and they're all a little it confusing. I'm not sure if I can use them with the STL, if I can't (although I believe someone said its safe to use these, and not auto_ptr?). Thanks for everyone's help.

Share this post


Link to post
Share on other sites
I think we need more information to resolve this problem. I tried to build up a minimal version of what you're doing based on what you're posted so far, and it works fine:
#include <deque>
#include <iostream>
#include <list>
#include <boost/bind.hpp>
#include <boost/shared_ptr.hpp>

class ITask
{

public:

virtual void exec() const = 0;

};

class TimerTask
:
public ITask
{

public:

virtual void exec() const
{
std::cout << "TimerTask\n";
}

};

class VideoTask
:
public ITask
{

public:

virtual void exec() const
{
std::cout << "VideoTask\n";
}

};

class LogicTask
:
public ITask
{

public:

virtual void exec() const
{
std::cout << "LogicTask\n";
}

};

typedef std::list< boost::shared_ptr< ITask > > CTaskList;

class CKernel
{

public:

void boot(CTaskList taskList);
void run();
void LINK_TASK_TO_KERNEL(boost::shared_ptr< ITask >);

private:

CTaskList m_TaskList;
std::deque< boost::shared_ptr< ITask > > tasks_;

};

class CEngine
{

public:

CEngine();

private:

boost::shared_ptr< TimerTask > m_pTimer;
boost::shared_ptr< VideoTask > m_pVideo;
boost::shared_ptr< LogicTask > m_pLogic;
boost::shared_ptr< CKernel > m_pKernel;

};

void CKernel::boot(CTaskList taskList)
{

m_TaskList = taskList;

for(CTaskList::iterator it = m_TaskList.begin(); it != m_TaskList.end(); ++it)
{
LINK_TASK_TO_KERNEL(*it);
}
run();
}

void CKernel::run()
{
std::for_each(tasks_.begin(), tasks_.end(), boost::bind(&ITask::exec, _1));
}

void CKernel::LINK_TASK_TO_KERNEL(boost::shared_ptr< ITask > task)
{
tasks_.push_back(task);
}

CEngine::CEngine()
:
m_pTimer(new TimerTask()),
m_pVideo(new VideoTask()),
m_pLogic(new LogicTask())
{
m_pKernel = boost::shared_ptr<CKernel> (new CKernel);
// log = shared_ptr<logFile> (new logFile("log.txt"));

CTaskList taskList; // used to pass into the kernel booting process

// we need to add the tasks to the task list in order for the kernel to run
// them. this makes everything as abstract as possible so we can cut and paste
taskList.push_back( m_pTimer );
taskList.push_back( m_pVideo );
taskList.push_back( m_pLogic );

m_pKernel->boot( taskList );
// *** we're running at this point, and should only exit afrer the loop dies ***
}

int main()
{
CEngine e;
}

Perhaps you could post anything that looks structuraly very different in your actual code? Also, did you actually check that m_pTimer were actually set when you added them to the list (i.e. the owned pointer was not NULL)?

Finally, yes, you can use boost::shared_ptr with the SC++L (Standard C++ Library, abbreviation stolen from MaulingMonkey). The SC++L container classes require that copies are equivalent. For boost::shared_ptr this is true. Copy a boost::shared_ptr and both the original and the copy point to the same object. For std::auto_ptr on the other hand copying transfers ownership. Copy a std::auto_ptr and the copy points to the object while the original points to nothing.

Enigma

Share this post


Link to post
Share on other sites
Ahh, yeah, I feel like an idiot now. When I switched to using shared_ptr I didn't do this:

m_pTimer = shared_ptr<TimerTask> (new TimerTask(30) );
m_pLogic = shared_ptr<LogicTask> (new LogicTask(20) );
m_pVideo = shared_ptr<VideoTask> (new VideoTask(10) );

I actually left to run some errands and came back, immediately when I saw it I slapped myself in the forehead. Mistakes, mistakes. Thanks everyone for their help.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this