Skip to content

Commit

Permalink
incorporated libxml "wrapper node" memory leak fixes from
Browse files Browse the repository at this point in the history
28e90bb
(behind a preprocessor #ifdef which is inactive by default, as I am not
100% sure the proposed changes were tested on all platforms ... we are
certainly having segmentation fault issues on at least Android NDK / JNI
with Gcc and GNU STL, related to std::shared_ptr<>, according to the stack
traces)
  • Loading branch information
danielweck committed Oct 28, 2016
1 parent efc71c2 commit a33c979
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 10 deletions.
29 changes: 27 additions & 2 deletions ePub3/xml/tree/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,13 @@ Node::Node(const string & name, NodeType type, const string & content, const cla
Node::Node(Node && o) : _xml(o._xml) {
typedef LibXML2Private<Node> _Private;
_Private* priv = reinterpret_cast<_Private*>(_xml->_private);

#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
priv->__ptr = std::shared_ptr<Node>(this);
#else
priv->__ptr.reset(this);
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER

o._xml = NULL;
}
Node::~Node()
Expand All @@ -181,14 +187,31 @@ Node::~Node()
return;

_Private* priv = reinterpret_cast<_Private*>(_xml->_private);
if ( priv->__sig != _READIUM_XML_SIGNATURE || priv->__ptr.get() != this )
if ( priv->__sig != _READIUM_XML_SIGNATURE ||
#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
(priv->__ptr.lock() != nullptr && priv->__ptr.lock().get() != this)
#else
priv->__ptr.get() != this
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER
) {
return;

}

#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
if (!bool(priv->__ptr.lock()))
{
_xml->_private = nullptr;
delete priv;
}
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER

// free the underlying node if *and only if* it is detached
if ( _xml->parent == nullptr && _xml->prev == nullptr && _xml->next == nullptr )
{
#if !ENABLE_WEAK_PTR_XML_NODE_WRAPPER
_xml->_private = nullptr;
delete priv;
#endif //!ENABLE_WEAK_PTR_XML_NODE_WRAPPER
xmlFreeNode(_xml);
}
}
Expand Down Expand Up @@ -646,7 +669,9 @@ void Node::Unwrap(_xmlNode *aNode)
NsPrivate* ptr = reinterpret_cast<NsPrivate*>(__ns->_private);
if (ptr->__sig == _READIUM_XML_SIGNATURE)
{
#if !ENABLE_WEAK_PTR_XML_NODE_WRAPPER
ptr->__ptr->release();
#endif //!ENABLE_WEAK_PTR_XML_NODE_WRAPPER
delete ptr;
}
__ns->_private = nullptr;
Expand Down
64 changes: 57 additions & 7 deletions ePub3/xml/utilities/base.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,13 @@ struct LibXML2Private
: __sig(_READIUM_XML_SIGNATURE), __ptr(nullptr)
{}
LibXML2Private(_Tp* __p)
: __sig(_READIUM_XML_SIGNATURE), __ptr(__p)
: __sig(_READIUM_XML_SIGNATURE), __ptr(
#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
std::shared_ptr<_Tp>(__p)
#else
__p
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER
)
{}
LibXML2Private(std::shared_ptr<_Tp>& __p)
: __sig(_READIUM_XML_SIGNATURE), __ptr(__p)
Expand All @@ -133,7 +139,12 @@ struct LibXML2Private

// data member-- used to determine if this is a Readium-made pointer
unsigned int __sig;
std::shared_ptr<_Tp> __ptr;
#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
std::weak_ptr<_Tp> __ptr;
#else
std::shared_ptr<_Tp> __ptr;
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER

};
#endif

Expand Down Expand Up @@ -162,7 +173,26 @@ static inline std::shared_ptr<_Tp> Wrapped(_Nm * __n)
_PrivatePtr __p = reinterpret_cast<_PrivatePtr>(__n->_private);
if (__p->__sig == _READIUM_XML_SIGNATURE)
{
#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
std::shared_ptr<_Tp> toRet = __p->__ptr.lock();
if (!bool(toRet))// if there is no live reference to the readium wrapper, but
{
delete __p; // release unreferenced memory, if any
__n->_private = nullptr;

std::shared_ptr<_Tp> toRet = std::shared_ptr<_Tp>(new _Tp(__n));

//_PrivatePtr __p = new LibXML2Private<_Tp>(new _Tp(__n));
_PrivatePtr __p = new LibXML2Private<_Tp>(toRet);
__n->_private = __p;
//return __p->__ptr;
return toRet;
}

return toRet;
#else
return __p->__ptr;
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER
}
else
{
Expand All @@ -175,11 +205,25 @@ static inline std::shared_ptr<_Tp> Wrapped(_Nm * __n)
if (__n->_private != nullptr)
throw std::logic_error("XML _private already carries a value!");
}


_PrivatePtr __p = new LibXML2Private<_Tp>(new _Tp(__n));

#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
std::shared_ptr<_Tp> toRet = std::shared_ptr<_Tp>(new _Tp(__n));
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER

_PrivatePtr __p = new LibXML2Private<_Tp>(
#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
toRet
#else
new _Tp(__n)
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER
);
__n->_private = __p;
return __p->__ptr;
return
#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
toRet;
#else
__p->__ptr;
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER
}

/**
Expand All @@ -198,7 +242,13 @@ static inline void Rewrap(_Nm* __n, std::shared_ptr<_Tp> __t)
if (IS_READIUM_WRAPPED_XML(__n))
{
_Private* __p = reinterpret_cast<_Private*>(__n->_private);
if (__p->__ptr == __t)
if (
#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
__p->__ptr.lock() && __p->__ptr.lock() == __t
#else
__p->__ptr == __t
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER
)
return;

delete __p;
Expand Down
8 changes: 7 additions & 1 deletion ePub3/xml/validation/ns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ Namespace::~Namespace()
return;

LibXML2Private<Namespace>* priv = reinterpret_cast<LibXML2Private<Namespace>*>(_xml->_private);
if (priv->__sig == _READIUM_XML_SIGNATURE && priv->__ptr.get() == this)
if (
#if ENABLE_WEAK_PTR_XML_NODE_WRAPPER
priv->__sig == _READIUM_XML_SIGNATURE && priv->__ptr.lock() == nullptr/* && priv->__ptr.lock().get() == this*/
#else
priv->__sig == _READIUM_XML_SIGNATURE && priv->__ptr.get() == this
#endif //ENABLE_WEAK_PTR_XML_NODE_WRAPPER
)
{
delete priv;
_xml->_private = nullptr;
Expand Down

0 comments on commit a33c979

Please sign in to comment.