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.
_______________________________________________ PyKDE mailing list PyKDE@mats.imk.fraunhofer.de http://mats.imk.fraunhofer.de/mailman/listinfo/pykde
