[PyKDE] boost::shared_ptr

[James Emerton]

On 22-Mar-05, at 12:34 PM, Jim Bublitz wrote:

>> It appears that when passing Python instances back into C++ you are
>> creating a second, unrelated smart pointer.  Unless KSharedPtr is
>> maintaining a mapping of C++ pointers, the pointer that gets passed in
>> is getting an extra reference added to it that will never go away.
>> When the pointer is passed back out of Python, you area creating an
>> unassociated smart pointer that will deallocate the contained pointer
>> when it goes out of scope, potentially leaving a dangling pointer.
>
> Yeah - I forgot about that part (or ignorant of it - not sure which). 
> The
> problem is that incrementing the ref count of the original pointer 
> (sipCpp)
> has the same result, and the underlying object doesn't have a copy
> constructor (I believe boost works that way too).

boost::shared_ptr relies on the copy ctor and assignment op to copy the 
internal reference count.

I've just had a look at the KSharedPtr docs, and it appears that the 
creation of unrelated KSharedPtrs is handled by the implementation of 
KShared.  KSharedPtr embeds the reference count *inside* the class it's 
wrapping.  The advantage of this is that you cannot shoot yourself in 
the foot with something like KSharedPtr<T> foo( bar.data() ).  (Where 
bar is another KSharedPtr<T> instance.)  The disadvantage is that you 
must change the interface of the wrapped class.  (Less than desirable 
in some cases.)

The boost implementation doesn't require modification of the wrapped 
class, but uses an embedded reference count object, allocated on the 
heap.  This means that if you do boost::shared_ptr<T> foo( bar.get() ) 
you *will* end up with a dangling pointer in bar when foo goes out of 
scope (or vice versa.)

Anyway, I retract my previous statement, your original %MappedType code 
will work nicely for KSharedPtr.  Not helping me though!  =P

> It seems like you should be able to subclass the shared ptr and bind 
> the
> subclass instead, overload the C++ dtor to Py_DECREF the corresponding 
> Python
> object (need to keep a ptr to it) and overload __del__ to decrement 
> the ref
> count of the C++ object.

Well, shared_ptr is not designed to be subclassed.  (nonvirtual 
destructor)  And I don't really want to make such an intrusive change.


I have worked on an implementation I alluded to in a previous message.  
I think this might be along the right lines, it's included below.  A 
more refined implementation would probably use a single map with void 
pointers and template function wrappers, mainly to avoid code bloat.  
Hopefully someone finds this useful, and I'll post a final version when 
it's done.  (It would be a lot cleaner if there were somewhere better 
to put the shared_ptr instance; like inside the PyObject.)

The problem I'm having with this now is that I am not releasing a 
reference.  BVApp_ISalesOrder::__del__ in defined in the wrapper to 
call shared_ptr_mgr::release_ref()  The only way I have found so far to 
force __del__ to be called is by calling it explicitly in Python.  I 
have tried using 'del' on the builtin and then calling gc.collect() and 
it will not go away.  =(  Perhaps this has something to do with the 
'Abstract' annotation on the class?


James



%ModuleHeaderCode
#include <map>
#include <utility>
#include <boost/shared_ptr.hpp>

template< class T >
struct shared_ptr_mgr
{
	typedef T										cpp_type;
	typedef boost::shared_ptr< cpp_type >			smart_pointer_type;
	typedef std::pair< smart_pointer_type, int >	smart_pointer_count;
	typedef std::map< cpp_type*, smart_pointer_count >	pointer_map_type;

	static pointer_map_type* _pointer_map;

	static void init()
	{
		if( !_pointer_map )
			_pointer_map = new pointer_map_type();
	}

	// Add a new reference or increment a previous reference
	static int add_ref( smart_pointer_type& ref )
	{
		init();

		pointer_map_type::iterator it = _pointer_map->find( ref.get() );
		if( it != _pointer_map->end() )
			return ++(*it).second.second;

		_pointer_map->insert(std::make_pair( ref.get(), std::make_pair(ref, 
1) ));
		return 1;
	}

	// Release a reference
	static int release_ref( cpp_type* ptr )
	{
		init();

		pointer_map_type::iterator it = _pointer_map->find( ptr );
		if( it == _pointer_map->end() )
			return -1;

		int cnt = --(*it).second.second;
		if( 0 == cnt )
			_pointer_map->erase( it );

		return cnt;
	}

	// Create a new smart pointer instance (from copy in map if available)
	static smart_pointer_type* create_pointer( cpp_type* ptr )
	{
		init();
		std::cout << "create_pointer():" << std::hex << ptr << std::endl;

		pointer_map_type::iterator it = _pointer_map->find( ptr );
		if( it == _pointer_map->end() )
			return new smart_pointer_type( ptr );
		else
			return new smart_pointer_type( (*it).second.first );
	}
};

%End

%ModuleCode
// Need to add one of these for each pointer type (in this incarnation, 
at least)
shared_ptr_mgr< BVApp::ISalesOrder >::pointer_map_type* shared_ptr_mgr< 
BVApp::ISalesOrder >::_pointer_map( NULL );
%End

%MappedType BVApp_ISalesOrderPtr
{
%TypeHeaderCode
#include <BVData/SalesOrder.h>
#include "sipbvBVApp_ISalesOrder.h"

typedef BVApp::ISalesOrderPtr BVApp_ISalesOrderPtr;
%End

%ConvertFromTypeCode
	if( !sipCpp )
		return NULL;

	// Add an extra reference...
	shared_ptr_mgr< BVApp::ISalesOrder >::add_ref( *sipCpp );

	PyObject* o = sipMapCppToSelf( sipCpp->get(), 
sipClass_BVApp_ISalesOrder );
	return o;
%End

%ConvertToTypeCode
	if( sipIsErr == NULL )
		return PyInstance_Check( sipPy );

	int iserr = 0;
	BVApp::ISalesOrder* ord = reinterpret_cast<BVApp::ISalesOrder*>(
		sipForceConvertTo_BVApp_ISalesOrder( sipPy, &iserr ) );

	if( iserr )
	{
		*sipIsErr = 1;
		return 0;
	}

	*sipCppPtr = shared_ptr_mgr< BVApp::ISalesOrder >::create_pointer( ord 
);
	return 1;
%End
};


--
This is not my home; the cats just let me stay here.