Re: [boost] boost::thread question?

From: Maxim Yegorushkin (maxim.yegorushkin_at_[hidden])
Date: 2006-08-23 11:16:42

jmzorko_at_[hidden] wrote:

> OK, i'm experiencing a crash every now and then (WIndows) and all-the- 
> freakin'-time (Mac OSX) for my app.  Looking into it, I see this,  
> which I wrote before I understood the beauty of scoped_ptr:
> 
> template< class T > class Thread
> {
> public:
> 	Thread() : pBoostThread( NULL ), pThreadFunction( NULL ),  
> m_ExitSignal( false ) {}

You could save some typing doing:

     Thread() : pBoostThread(), pThreadFunction(), m_ExitSignal() {}

> 	return_status Init( return_status ( *pFunc )( Thread * ), T varArg )
> 	{
> 		pThreadFunction = pFunc;
> 		varThreadArg = varArg;
> 		return SUCCESS;
> 	}

You may not need return_status here because assigning built-in types never 
fails. Should it fail you get a core dump.

> 	return_status Start()
> 	{
> 		pBoostThread = new boost::thread( boost::bind( pThreadFunction,  
> this ) );
> 		return SUCCESS;
> 	}

No need for return_status either. If new boost::thread(...) fails it throws, so 
this function either returns SUCCESS or throws an exception. The former conveys 
no information.

> 	return_status Exit( unsigned long exit_code )
> 	{
> 		if ( pBoostThread ) delete pBoostThread;	// ouch!!!

You don't need to check the pointer for null before delete/free, they do that 
for you.

If this line does ouch!!!, this might be because you did not implement or 
prohibited copying your Thread<> objects, or you call Exit several times and it 
does not do pBoostThread = 0 after deleting, so that it ouch!!!es when you 
delete the same pBoostThread more then once.

Note that destroying a boost::thread object does not destroy the thread. It 
merely makes it detached.

> 	}
> };
> 
> I tried wrapping the icky "new boost::thread< ... >" in a  
> boost::scoped_ptr, like so:
> 
> 		pBoostThread = new boost::scoped_ptr< boost::thread( boost::bind 
> ( pThreadFunction, this ) ) >;

The proper syntax would be:

     boost::scoped_ptr<boost::thread> p = new boost::thread(...);

You need boost::scoped_ptr<boost::thread> as a member of your class instead of 
pBoostThread plain pointer. Note, that by declaring 
boost::scoped_ptr<boost::thread> pBoostThread as a member, you also make your 
class non-copyable as boost::scoped_ptr<> is not copyable.

You could take a 30-minute brake, get yourself your favorite drink, and peruse 
boost::thread documentation and examples, they are easy to follow. You might 
like to think again why you need Thread<>, as judging from what was posted it 
adds nothing to what boost::thread already provides.