From 4f68489ae200d31be8f53fb1818abb2c9427611d Mon Sep 17 00:00:00 2001 From: Caolán McNamara Date: Wed, 2 Feb 2011 13:12:46 +0000 Subject: Fix per-frame leak This was defined as a uno listener, but wasn't registered as a uno listener, but was a vcl listener. There was no route for it to be released despite all the optimistic comments. --- framework/inc/services/frame.hxx | 4 ++ .../source/dispatch/windowcommanddispatch.cxx | 43 +++++++++++++++------- .../source/inc/dispatch/windowcommanddispatch.hxx | 24 +++++------- framework/source/services/frame.cxx | 6 ++- 4 files changed, 47 insertions(+), 30 deletions(-) (limited to 'framework') diff --git a/framework/inc/services/frame.hxx b/framework/inc/services/frame.hxx index fc559880107e..7edb072493b9 100644 --- a/framework/inc/services/frame.hxx +++ b/framework/inc/services/frame.hxx @@ -105,6 +105,8 @@ enum EActiveState E_FOCUS // I have the focus now. I must a member of an active path! }; +class WindowCommandDispatch; + //_________________________________________________________________________________________________________________ // exported definitions //_________________________________________________________________________________________________________________ @@ -438,6 +440,8 @@ class Frame : // interfaces css::uno::Reference< css::frame::XDispatchInformationProvider > m_xDispatchInfoHelper ; css::uno::Reference< css::frame::XTitle > m_xTitleHelper ; + WindowCommandDispatch* m_pWindowCommandDispatch ; + protected: FrameContainer m_aChildFrameContainer ; /// array of child frames diff --git a/framework/source/dispatch/windowcommanddispatch.cxx b/framework/source/dispatch/windowcommanddispatch.cxx index 09efdb3e8ae6..459496fd300e 100644 --- a/framework/source/dispatch/windowcommanddispatch.cxx +++ b/framework/source/dispatch/windowcommanddispatch.cxx @@ -82,30 +82,20 @@ WindowCommandDispatch::WindowCommandDispatch(const css::uno::Reference< css::lan //----------------------------------------------- WindowCommandDispatch::~WindowCommandDispatch() { + impl_stopListening(); m_xSMGR.clear(); } -//----------------------------------------------- -void SAL_CALL WindowCommandDispatch::disposing(const css::lang::EventObject& /*aSource*/) - throw (css::uno::RuntimeException) -{ - // We hold our window weak ... so there is no need to clear it's reference here. - // The window and we will die by ref count automatically. -} - //----------------------------------------------- void WindowCommandDispatch::impl_startListening() { - // SYNCHRONIZED -> ReadGuard aReadLock(m_aLock); css::uno::Reference< css::awt::XWindow > xWindow( m_xWindow.get(), css::uno::UNO_QUERY ); aReadLock.unlock(); - // <- SYNCHRONIZED if ( ! xWindow.is()) return; - // SYNCHRONIZED -> { SolarMutexGuard aSolarLock; @@ -115,9 +105,31 @@ void WindowCommandDispatch::impl_startListening() pWindow->AddEventListener( LINK(this, WindowCommandDispatch, impl_notifyCommand) ); } - // <- SYNCHRONIZED } +void WindowCommandDispatch::impl_stopListening() +{ + ReadGuard aReadLock(m_aLock); + css::uno::Reference< css::awt::XWindow > xWindow( m_xWindow.get(), css::uno::UNO_QUERY ); + aReadLock.unlock(); + + if (!xWindow.is()) + return; + + { + SolarMutexGuard aSolarLock; + + Window* pWindow = VCLUnoHelper::GetWindow(xWindow); + if (!pWindow) + return; + + pWindow->RemoveEventListener( LINK(this, WindowCommandDispatch, impl_notifyCommand) ); + + m_xWindow.clear(); + } +} + + //----------------------------------------------- IMPL_LINK(WindowCommandDispatch, impl_notifyCommand, void*, pParam) { @@ -125,6 +137,11 @@ IMPL_LINK(WindowCommandDispatch, impl_notifyCommand, void*, pParam) return 0L; const VclWindowEvent* pEvent = (VclWindowEvent*)pParam; + if (pEvent->GetId() == VCLEVENT_OBJECT_DYING) + { + impl_stopListening(); + return 0L; + } if (pEvent->GetId() != VCLEVENT_WINDOW_COMMAND) return 0L; @@ -137,7 +154,7 @@ IMPL_LINK(WindowCommandDispatch, impl_notifyCommand, void*, pParam) return 0L; const int nCommand = pData->GetDialogId(); - ::rtl::OUString sCommand; + ::rtl::OUString sCommand; switch (nCommand) { diff --git a/framework/source/inc/dispatch/windowcommanddispatch.hxx b/framework/source/inc/dispatch/windowcommanddispatch.hxx index 03aded6ad1ad..549c6d9e69ae 100644 --- a/framework/source/inc/dispatch/windowcommanddispatch.hxx +++ b/framework/source/inc/dispatch/windowcommanddispatch.hxx @@ -67,13 +67,12 @@ namespace css = ::com::sun::star; e.g. "Pereferences" or "About". These menu entries trigger hard coded commands. Here we map these commands to the right URLs and dispatch them. - This helper knows a frame and it's container window (where VCL provide the hard coded - commands). We hold those objects weak ... so there is no need to react for complex dispose/ing() + This helper knows a frame and its container window (where VCL provide the hard coded + commands). We hold those objects weak so there is no need to react for complex UNO dispose/ing() scenarios. On the other side VCL does not hold us alive (because it doesn't know our UNO reference). - So we register us at the XWindow as event listener also to be sure to live as long the XWindow/VCLWindow lives. + So we register at the VCL level as an event listener and */ class WindowCommandDispatch : private ThreadHelpBase - , public ::cppu::WeakImplHelper1< css::lang::XEventListener > { //___________________________________________ // const @@ -128,15 +127,6 @@ class WindowCommandDispatch : private ThreadHelpBase */ virtual ~WindowCommandDispatch(); - //___________________________________________ - // uno interface - - public: - - // XEventListener - virtual void SAL_CALL disposing(const css::lang::EventObject& aSource) - throw (css::uno::RuntimeException); - //___________________________________________ // implementation @@ -148,11 +138,15 @@ class WindowCommandDispatch : private ThreadHelpBase @descr Those listener connections will be created one times only (see ctor). Afterwards we listen for incoming events till our referred frame/window pair - will be closed. All objects die by refcount automatically. Because we hold - it weak ... + will be closed. */ void impl_startListening(); + /** @short drop all listener connections we need. + + */ + void impl_stopListening(); + //_______________________________________ /** @short callback from VCL to notify new commands diff --git a/framework/source/services/frame.cxx b/framework/source/services/frame.cxx index 67adb5242c0c..12e44e57cb1f 100644 --- a/framework/source/services/frame.cxx +++ b/framework/source/services/frame.cxx @@ -292,6 +292,7 @@ Frame::Frame( const css::uno::Reference< css::lang::XMultiServiceFactory >& xFac , m_bSelfClose ( sal_False ) // Important! , m_bIsHidden ( sal_True ) , m_xTitleHelper ( ) + , m_pWindowCommandDispatch ( 0 ) , m_aChildFrameContainer ( ) { // Check incoming parameter to avoid against wrong initialization. @@ -622,8 +623,7 @@ void SAL_CALL Frame::initialize( const css::uno::Reference< css::awt::XWindow >& impl_enablePropertySet(); - // create WindowCommandDispatch; it is supposed to release itself at frame destruction - (void)new WindowCommandDispatch(xSMGR, this); + m_pWindowCommandDispatch = new WindowCommandDispatch(xSMGR, this); // Initialize title functionality TitleHelper* pTitleHelper = new TitleHelper(xSMGR); @@ -1863,6 +1863,8 @@ void SAL_CALL Frame::dispose() throw( css::uno::RuntimeException ) // We will die, die and die ... implts_stopWindowListening(); + delete m_pWindowCommandDispatch; + // Send message to all listener and forget her references. css::lang::EventObject aEvent( xThis ); m_aListenerContainer.disposeAndClear( aEvent ); -- cgit