From fcbce75165fb2bd3f62820e1bdc6429f784dd9af Mon Sep 17 00:00:00 2001 From: Ronan Abhamon Date: Mon, 6 Nov 2017 15:06:06 +0100 Subject: [PATCH] feat(c-wrapper): deal with floating references --- src/c-wrapper/internal/c-tools.h | 156 +++++++++++++++++++++++++------ src/object/base-object.cpp | 4 + 2 files changed, 130 insertions(+), 30 deletions(-) diff --git a/src/c-wrapper/internal/c-tools.h b/src/c-wrapper/internal/c-tools.h index 7ec402e3c..becf60c5a 100644 --- a/src/c-wrapper/internal/c-tools.h +++ b/src/c-wrapper/internal/c-tools.h @@ -121,10 +121,19 @@ private: // Wrapped Objects. // --------------------------------------------------------------------------- + enum class WrappedObjectOwner : int { + Internal, + External + }; + template struct WrappedBaseObject { belle_sip_object_t base; std::shared_ptr cppPtr; + std::weak_ptr weakCppPtr; + + // By default: Internal. + WrappedObjectOwner owner; }; template @@ -143,6 +152,23 @@ private: } public: + // --------------------------------------------------------------------------- + // Belle sip handlers. + // Deal with floating references. + // --------------------------------------------------------------------------- + + static void onBelleSipFirstRef (belle_sip_object_t *base) { + WrappedBaseObject *wrappedObject = reinterpret_cast *>(base); + if (wrappedObject->owner == WrappedObjectOwner::Internal) + wrappedObject->cppPtr = wrappedObject->weakCppPtr.lock(); + } + + static void onBelleSipLastRef (belle_sip_object_t *base) { + WrappedBaseObject *wrappedObject = reinterpret_cast *>(base); + if (wrappedObject->owner == WrappedObjectOwner::Internal) + wrappedObject->cppPtr.reset(); + } + // --------------------------------------------------------------------------- // Casts. // --------------------------------------------------------------------------- @@ -178,6 +204,20 @@ public: return cppObject->getPrivate(); } + // --------------------------------------------------------------------------- + // Deal with cpp ptr destruction. + // --------------------------------------------------------------------------- + + template< + typename CppType, + typename = typename std::enable_if::value, CppType>::type + > + static void signalCppPtrDestruction (CppType *cppObject) { + void *value = cppObject->getCBackPtr(); + if (value && static_cast *>(value)->owner == WrappedObjectOwner::Internal) + belle_sip_object_unref(value); + } + // --------------------------------------------------------------------------- // Get c/cpp ptr helpers. // --------------------------------------------------------------------------- @@ -187,12 +227,19 @@ public: typename CppType = typename CTypeMetaInfo::cppType, typename = typename std::enable_if::value, CppType>::type > - static L_INTERNAL_WRAPPER_CONSTEXPR std::shared_ptr getCppPtrFromC (CType *cObject) { + static inline std::shared_ptr getCppPtrFromC (CType *cObject) { #ifdef DEBUG typedef typename CTypeMetaInfo::cppType BaseType; typedef CppType DerivedType; - std::shared_ptr cppObject = reinterpret_cast *>(cObject)->cppPtr; + std::shared_ptr cppObject; + { + WrappedBaseObject *wrappedObject = reinterpret_cast *>(cObject); + cppObject = wrappedObject->owner == WrappedObjectOwner::Internal + ? wrappedObject->weakCppPtr.lock() + : wrappedObject->cppPtr; + } + if (!cppObject) abort("Cpp Object is null."); @@ -202,7 +249,10 @@ public: return derivedCppObject; #else - return reinterpret_cast *>(cObject)->cppPtr; + WrappedBaseObject *wrappedObject = reinterpret_cast *>(cObject); + return wrappedObject->owner == WrappedObjectOwner::Internal + ? wrappedObject->weakCppPtr.lock() + : wrappedObject->cppPtr; #endif } @@ -211,12 +261,8 @@ public: typename CppType = typename CTypeMetaInfo::cppType, typename = typename std::enable_if::value, CppType>::type > - static L_INTERNAL_WRAPPER_CONSTEXPR std::shared_ptr getCppPtrFromC (const CType *cObject) { - #ifdef DEBUG - return getCppPtrFromC(const_cast(cObject)); - #else - return reinterpret_cast *>(cObject)->cppPtr; - #endif + static inline std::shared_ptr getCppPtrFromC (const CType *cObject) { + return getCppPtrFromC(const_cast(cObject)); } template< @@ -266,7 +312,23 @@ public: typename = typename std::enable_if::value, CppType>::type > static inline void setCppPtrFromC (CType *cObject, const std::shared_ptr &cppObject) { - reinterpret_cast *>(cObject)->cppPtr = cppObject; + WrappedBaseObject *wrappedObject = reinterpret_cast *>(cObject); + std::shared_ptr oldCppObject; + + if (wrappedObject->owner == WrappedObjectOwner::Internal) { + oldCppObject = wrappedObject->weakCppPtr.lock(); + wrappedObject->weakCppPtr = cppObject; + if (reinterpret_cast(cObject)->ref > 1) + wrappedObject->cppPtr = cppObject; + else + wrappedObject->cppPtr.reset(); + } else { + oldCppObject = wrappedObject->cppPtr; + wrappedObject->cppPtr = cppObject; + } + + if (oldCppObject) + oldCppObject->setCBackPtr(nullptr); cppObject->setCBackPtr(cObject); } @@ -295,7 +357,8 @@ public: } // --------------------------------------------------------------------------- - // Get c back ptr helpers. + // Get cpp ptr (shared if BaseObject, not shared if ClonableObject) + // from cpp object. // --------------------------------------------------------------------------- template< @@ -328,13 +391,23 @@ public: return cppObject; } + // --------------------------------------------------------------------------- + // Get c back ptr resolver helpers. + // --------------------------------------------------------------------------- + template< typename CppType, - typename = typename std::enable_if< - IsDefinedCppObject::value || IsDefinedClonableCppObject::value, CppType - >::type + typename = typename std::enable_if::value, CppType>::type > - static inline typename CppTypeMetaInfo::cType *getCBackPtr (const CppType *cppObject) { + static inline typename CppTypeMetaInfo::cType *getCBackPtrResolver (const std::shared_ptr &cppObject) { + return getCBackPtr(cppObject); + } + + template< + typename CppType, + typename = typename std::enable_if::value, CppType>::type + > + static inline typename CppTypeMetaInfo::cType *getCBackPtrResolver (const CppType *cppObject) { if (L_UNLIKELY(!cppObject)) return nullptr; @@ -345,13 +418,25 @@ public: return static_cast(value); RetType *cObject = CppTypeMetaInfo::init(); - - // Can be set only on Object or ClonableObject. Not BaseObject. - setCppPtrFromC(cObject, getResolvedCppPtr(cppObject)); + setCppPtrFromC(cObject, cppObject); return cObject; } + // --------------------------------------------------------------------------- + // Get c back ptr helpers. + // --------------------------------------------------------------------------- + + template< + typename CppType, + typename = typename std::enable_if< + IsDefinedCppObject::value || IsDefinedClonableCppObject::value, CppType + >::type + > + static inline typename CppTypeMetaInfo::cType *getCBackPtr (const CppType *cppObject) { + return getCBackPtrResolver(getResolvedCppPtr(cppObject)); + } + template< typename CppType, typename = typename std::enable_if::value, CppType>::type @@ -367,6 +452,7 @@ public: return static_cast(value); RetType *cObject = CppTypeMetaInfo::init(); + reinterpret_cast *>(cObject)->owner = WrappedObjectOwner::Internal; setCppPtrFromC(cObject, cppObject); @@ -470,25 +556,38 @@ LINPHONE_END_NAMESPACE #define L_INTERNAL_C_OBJECT_NO_XTOR(C_OBJECT) +#define L_INTERNAL_DECLARE_C_OBJECT(C_TYPE, CPP_TYPE, ...) \ + struct _Linphone ## C_TYPE { \ + belle_sip_object_t base; \ + std::shared_ptr cppPtr; \ + std::weak_ptr weakCppPtr; \ + int owner; \ + __VA_ARGS__ \ + }; + #define L_INTERNAL_DECLARE_C_OBJECT_FUNCTIONS(C_TYPE, CONSTRUCTOR, DESTRUCTOR) \ BELLE_SIP_DECLARE_VPTR_NO_EXPORT(Linphone ## C_TYPE); \ Linphone ## C_TYPE *_linphone_ ## C_TYPE ## _init () { \ Linphone ## C_TYPE * object = belle_sip_object_new(Linphone ## C_TYPE); \ new(&object->cppPtr) std::shared_ptr(); \ + new(&object->weakCppPtr) std::weak_ptr(); \ CONSTRUCTOR(object); \ return object; \ } \ static void _linphone_ ## C_TYPE ## _uninit(Linphone ## C_TYPE * object) { \ DESTRUCTOR(object); \ - object->cppPtr.~shared_ptr (); \ + object->cppPtr.~shared_ptr(); \ + object->weakCppPtr.~weak_ptr(); \ } \ BELLE_SIP_DECLARE_NO_IMPLEMENTED_INTERFACES(Linphone ## C_TYPE); \ - BELLE_SIP_INSTANCIATE_VPTR( \ + BELLE_SIP_INSTANCIATE_VPTR2( \ Linphone ## C_TYPE, \ belle_sip_object_t, \ _linphone_ ## C_TYPE ## _uninit, \ NULL, \ NULL, \ + LinphonePrivate::Wrapper::onBelleSipFirstRef, \ + LinphonePrivate::Wrapper::onBelleSipLastRef, \ FALSE \ ); @@ -551,21 +650,14 @@ LINPHONE_END_NAMESPACE // Declare wrapped C object with constructor/destructor. #define L_DECLARE_C_OBJECT_IMPL_WITH_XTORS(C_TYPE, CONSTRUCTOR, DESTRUCTOR, ...) \ - struct _Linphone ## C_TYPE { \ - belle_sip_object_t base; \ - std::shared_ptr cppPtr; \ - __VA_ARGS__ \ - }; \ + static_assert(LinphonePrivate::CTypeMetaInfo::defined, "Type is not defined."); \ + L_INTERNAL_DECLARE_C_OBJECT(C_TYPE, L_CPP_TYPE_OF_C_TYPE(C_TYPE), __VA_ARGS__) \ L_INTERNAL_DECLARE_C_OBJECT_FUNCTIONS(C_TYPE, CONSTRUCTOR, DESTRUCTOR) // Declare wrapped C object. #define L_DECLARE_C_OBJECT_IMPL(C_TYPE, ...) \ static_assert(LinphonePrivate::CTypeMetaInfo::defined, "Type is not defined."); \ - struct _Linphone ## C_TYPE { \ - belle_sip_object_t base; \ - std::shared_ptr cppPtr; \ - __VA_ARGS__ \ - }; \ + L_INTERNAL_DECLARE_C_OBJECT(C_TYPE, L_CPP_TYPE_OF_C_TYPE(C_TYPE), __VA_ARGS__) \ L_INTERNAL_DECLARE_C_OBJECT_FUNCTIONS(C_TYPE, L_INTERNAL_C_OBJECT_NO_XTOR, L_INTERNAL_C_OBJECT_NO_XTOR) // Declare clonable wrapped C object. @@ -607,6 +699,10 @@ LINPHONE_END_NAMESPACE // Call the init function of wrapped C object. #define L_INIT(C_TYPE) _linphone_ ## C_TYPE ## _init() +// Signal to wrapper the destruction of cpp base object. +#define L_SIGNAL_CPP_PTR_DESTRUCTION(CPP_OBJECT) \ + LinphonePrivate::Wrapper::signalCppPtrDestruction(CPP_OBJECT); + // Get/set the cpp-ptr of a wrapped C object. #define L_GET_CPP_PTR_FROM_C_OBJECT_1_ARGS(C_OBJECT) \ LinphonePrivate::Wrapper::getCppPtrFromC(C_OBJECT) diff --git a/src/object/base-object.cpp b/src/object/base-object.cpp index b1551c5ed..dca0953d5 100644 --- a/src/object/base-object.cpp +++ b/src/object/base-object.cpp @@ -19,6 +19,9 @@ #include "base-object-p.h" +// Necessary for: L_SIGNAL_CPP_PTR_DESTRUCTION. +#include "c-wrapper/internal/c-tools.h" + #include "base-object.h" // ============================================================================= @@ -32,6 +35,7 @@ BaseObject::BaseObject (BaseObjectPrivate &p) : mPrivate(&p) { } BaseObject::~BaseObject () { + L_SIGNAL_CPP_PTR_DESTRUCTION(this); delete mPrivate; }