Kaydet (Commit) a4928075 authored tarafından Caolán McNamara's avatar Caolán McNamara

fix memleak circular dependency of CElementList and CElement

launching impress leaks 70+k

==1458== 78,741 (152 direct, 78,589 indirect) bytes in 1 blocks are definitely lost in loss record 24,296 of 24,315
==1458==    at 0x4A0645D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1458==    by 0x4C3895D: rtl_allocateMemory_SYSTEM(unsigned long) (alloc_global.cxx:270)
==1458==    by 0x4C38A64: rtl_allocateMemory (alloc_global.cxx:303)
==1458==    by 0x2DCC0B67: cppu::OWeakObject::operator new(unsigned long) (weak.hxx:85)
==1458==    by 0x2DCCB3D3: DOM::CDocument::getElementsByTagName(rtl::OUString const&) (document.cxx:714)
==1458==    by 0x25DC99D6: SdDrawDocument::InitLayoutVector() (drawdoc.cxx:1008)

because the CElementList owns the CElement via m_pElement and
m_pElement owns the CElementList via the addEventListener.

Use a WeakEventListener pattern to let the CElement own
that helper which itself doesn't own the CElementList but is
owned by it instead, and forwards the events to the CElementList

In order to use that pattern the CElementList must be have a m_refCount
of 1 when the addEventListener is called, i.e. post ctor, so rename
the original CElementList as CElementListImpl and call its registerListener
from a wrapper CElementList

Change-Id: Ibd4f19b619543a4ef580366c69efb61b526696ab
üst bcbd2d65
......@@ -28,6 +28,34 @@ using namespace css::uno;
using namespace css::xml::dom;
using namespace css::xml::dom::events;
namespace
{
class WeakEventListener : public ::cppu::WeakImplHelper1<css::xml::dom::events::XEventListener>
{
private:
css::uno::WeakReference<css::xml::dom::events::XEventListener> mxOwner;
public:
WeakEventListener(const css::uno::Reference<css::xml::dom::events::XEventListener>& rOwner)
: mxOwner(rOwner)
{
}
virtual ~WeakEventListener()
{
}
virtual void SAL_CALL handleEvent(const css::uno::Reference<css::xml::dom::events::XEvent>& rEvent)
throw(css::uno::RuntimeException, std::exception) SAL_OVERRIDE
{
css::uno::Reference<css::xml::dom::events::XEventListener> xOwner(mxOwner.get(),
css::uno::UNO_QUERY);
if (xOwner.is())
xOwner->handleEvent(rEvent);
}
};
}
namespace DOM
{
......@@ -43,25 +71,46 @@ namespace DOM
CElementList::CElementList(::rtl::Reference<CElement> const& pElement,
::osl::Mutex & rMutex,
OUString const& rName, OUString const*const pURI)
: m_xImpl(new CElementListImpl(pElement, rMutex, rName, pURI))
{
if (pElement.is()) {
m_xImpl->registerListener(*pElement);
}
}
CElementListImpl::CElementListImpl(::rtl::Reference<CElement> const& pElement,
::osl::Mutex & rMutex,
OUString const& rName, OUString const*const pURI)
: m_pElement(pElement)
, m_rMutex(rMutex)
, m_pName(lcl_initXmlString(rName))
, m_pURI((pURI) ? lcl_initXmlString(*pURI) : 0)
, m_bRebuild(true)
{
if (m_pElement.is()) {
registerListener(*m_pElement);
}
CElementListImpl::~CElementListImpl()
{
if (m_xEventListener.is() && m_pElement.is())
{
Reference< XEventTarget > xTarget(static_cast<XElement*>(m_pElement.get()), UNO_QUERY);
assert(xTarget.is());
if (!xTarget.is())
return;
bool capture = false;
xTarget->removeEventListener("DOMSubtreeModified", m_xEventListener, capture);
}
}
void CElementList::registerListener(CElement & rElement)
void CElementListImpl::registerListener(CElement & rElement)
{
try {
Reference< XEventTarget > const xTarget(
static_cast<XElement*>(& rElement), UNO_QUERY_THROW);
bool capture = false;
m_xEventListener = new WeakEventListener(this);
xTarget->addEventListener("DOMSubtreeModified",
Reference< XEventListener >(this), capture);
m_xEventListener, capture);
} catch (const Exception &e){
OString aMsg("Exception caught while registering NodeList as listener:\n");
aMsg += OUStringToOString(e.Message, RTL_TEXTENCODING_ASCII_US);
......@@ -69,7 +118,7 @@ namespace DOM
}
}
void CElementList::buildlist(xmlNodePtr pNode, bool start)
void CElementListImpl::buildlist(xmlNodePtr pNode, bool start)
{
// bail out if no rebuild is needed
if (start) {
......@@ -107,7 +156,7 @@ namespace DOM
/**
The number of nodes in the list.
*/
sal_Int32 SAL_CALL CElementList::getLength() throw (RuntimeException, std::exception)
sal_Int32 SAL_CALL CElementListImpl::getLength() throw (RuntimeException, std::exception)
{
::osl::MutexGuard const g(m_rMutex);
......@@ -120,7 +169,7 @@ namespace DOM
/**
Returns the indexth item in the collection.
*/
Reference< XNode > SAL_CALL CElementList::item(sal_Int32 index)
Reference< XNode > SAL_CALL CElementListImpl::item(sal_Int32 index)
throw (RuntimeException, std::exception)
{
if (index < 0) throw RuntimeException();
......@@ -139,7 +188,7 @@ namespace DOM
}
// tree mutations can change the list
void SAL_CALL CElementList::handleEvent(Reference< XEvent > const&)
void SAL_CALL CElementListImpl::handleEvent(Reference< XEvent > const&)
throw (RuntimeException, std::exception)
{
::osl::MutexGuard const g(m_rMutex);
......
......@@ -36,6 +36,7 @@
#include <com/sun/star/xml/dom/events/XEvent.hpp>
#include <com/sun/star/xml/dom/events/XEventListener.hpp>
#include <cppuhelper/implbase1.hxx>
#include <cppuhelper/implbase2.hxx>
namespace DOM
......@@ -44,11 +45,16 @@ namespace DOM
typedef std::vector< xmlNodePtr > nodevector_t;
class CElementList
class CElementListImpl
: public cppu::WeakImplHelper2< css::xml::dom::XNodeList,
css::xml::dom::events::XEventListener >
{
private:
/** @short proxy weak binding to forward Events to ourself without
an ownership cycle
*/
css::uno::Reference< css::xml::dom::events::XEventListener > m_xEventListener;
::rtl::Reference<CElement> const m_pElement;
::osl::Mutex & m_rMutex;
::boost::scoped_array<xmlChar> const m_pName;
......@@ -57,13 +63,16 @@ namespace DOM
nodevector_t m_nodevector;
void buildlist(xmlNodePtr pNode, bool start=true);
void registerListener(CElement & rElement);
public:
CElementList(::rtl::Reference<CElement> const& pElement,
CElementListImpl(::rtl::Reference<CElement> const& pElement,
::osl::Mutex & rMutex,
OUString const& rName, OUString const*const pURI = 0);
void registerListener(CElement & rElement);
~CElementListImpl();
/**
The number of nodes in the list.
*/
......@@ -78,6 +87,41 @@ namespace DOM
virtual void SAL_CALL handleEvent(const css::uno::Reference< css::xml::dom::events::XEvent >& evt)
throw (css::uno::RuntimeException, std::exception) SAL_OVERRIDE;
};
class CElementList
: public cppu::WeakImplHelper2< css::xml::dom::XNodeList,
css::xml::dom::events::XEventListener >
{
private:
rtl::Reference<CElementListImpl> m_xImpl;
public:
CElementList(::rtl::Reference<CElement> const& pElement,
::osl::Mutex & rMutex,
OUString const& rName, OUString const*const pURI = 0);
/**
The number of nodes in the list.
*/
virtual sal_Int32 SAL_CALL getLength() throw (css::uno::RuntimeException, std::exception) SAL_OVERRIDE
{
return m_xImpl->getLength();
}
/**
Returns the indexth item in the collection.
*/
virtual css::uno::Reference< css::xml::dom::XNode > SAL_CALL item(sal_Int32 index)
throw (css::uno::RuntimeException, std::exception) SAL_OVERRIDE
{
return m_xImpl->item(index);
}
// XEventListener
virtual void SAL_CALL handleEvent(const css::uno::Reference< css::xml::dom::events::XEvent >& evt)
throw (css::uno::RuntimeException, std::exception) SAL_OVERRIDE
{
m_xImpl->handleEvent(evt);
}
};
}
#endif
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment