From 5aa60be574ece81b27c8f63e6e809871c694dba0 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Wed, 12 May 2021 11:33:06 +0200 Subject: fix leak in VCLXWindow which is a little tricky because dispose() can be called from either side (vcl::Window or VCLXWindow) Change-Id: Ifc380feec6bec84b2cf7d903d42db00755d4cd97 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115441 Tested-by: Jenkins Reviewed-by: Noel Grandin --- toolkit/source/awt/vclxwindow.cxx | 48 ++++++++++++++++++++++++++------------- vcl/source/window/window.cxx | 11 +++++++-- vcl/unx/gtk3/gtkframe.cxx | 10 +++++--- 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/toolkit/source/awt/vclxwindow.cxx b/toolkit/source/awt/vclxwindow.cxx index 4649a146d6e7..dfc6ba8f7c00 100644 --- a/toolkit/source/awt/vclxwindow.cxx +++ b/toolkit/source/awt/vclxwindow.cxx @@ -326,14 +326,7 @@ VCLXWindow::VCLXWindow( bool _bWithDefaultProps ) VCLXWindow::~VCLXWindow() { - mpImpl.reset(); - - if ( GetWindow() ) - { - GetWindow()->RemoveEventListener( LINK( this, VCLXWindow, WindowEventListener ) ); - GetWindow()->SetWindowPeer( nullptr, nullptr ); - GetWindow()->SetAccessible( nullptr ); - } + assert(!mpImpl && "forgot to call dispose()"); } @@ -379,7 +372,7 @@ void VCLXWindow::resumeVclEventListening( ) void VCLXWindow::notifyWindowRemoved( vcl::Window const & _rWindow ) { - if ( mpImpl->getContainerListeners().getLength() ) + if ( mpImpl && mpImpl->getContainerListeners().getLength() ) { awt::VclContainerEvent aEvent; aEvent.Source = *this; @@ -904,19 +897,25 @@ void VCLXWindow::dispose( ) { SolarMutexGuard aGuard; - mpImpl->mxViewGraphics = nullptr; + if (!mpImpl) + return; if ( mpImpl->mbDisposing ) return; + mpImpl->mxViewGraphics = nullptr; + mpImpl->mbDisposing = true; mpImpl->disposing(); - if ( GetWindow() ) + if ( auto pWindow = GetWindow() ) { + pWindow->RemoveEventListener( LINK( this, VCLXWindow, WindowEventListener ) ); + pWindow->SetWindowPeer( nullptr, nullptr ); + pWindow->SetAccessible( nullptr ); + VclPtr pOutDev = GetOutputDevice(); - SetWindow( nullptr ); // so that handlers are logged off, if necessary (virtual) SetOutputDevice( nullptr ); pOutDev.disposeAndClear(); } @@ -934,22 +933,23 @@ void VCLXWindow::dispose( ) { OSL_FAIL( "VCLXWindow::dispose: could not dispose the accessible context!" ); } - mpImpl->mxAccessibleContext.clear(); - mpImpl->mbDisposing = false; + mpImpl.reset(); } void VCLXWindow::addEventListener( const css::uno::Reference< css::lang::XEventListener >& rxListener ) { SolarMutexGuard aGuard; - + if (!mpImpl) // called during dispose by accessibility stuff + return; mpImpl->getEventListeners().addInterface( rxListener ); } void VCLXWindow::removeEventListener( const css::uno::Reference< css::lang::XEventListener >& rxListener ) { SolarMutexGuard aGuard; - + if (!mpImpl) + return; mpImpl->getEventListeners().removeInterface( rxListener ); } @@ -1036,6 +1036,9 @@ void VCLXWindow::removeWindowListener( const css::uno::Reference< css::awt::XWin { SolarMutexGuard aGuard; + if (!mpImpl) + return; + Reference< XWindowListener2 > xListener2( rxListener, UNO_QUERY ); if ( xListener2.is() ) mpImpl->getWindow2Listeners().removeInterface( xListener2 ); @@ -1052,6 +1055,8 @@ void VCLXWindow::addFocusListener( const css::uno::Reference< css::awt::XFocusLi void VCLXWindow::removeFocusListener( const css::uno::Reference< css::awt::XFocusListener >& rxListener ) { SolarMutexGuard aGuard; + if (!mpImpl) + return; mpImpl->getFocusListeners().removeInterface( rxListener ); } @@ -1064,6 +1069,8 @@ void VCLXWindow::addKeyListener( const css::uno::Reference< css::awt::XKeyListen void VCLXWindow::removeKeyListener( const css::uno::Reference< css::awt::XKeyListener >& rxListener ) { SolarMutexGuard aGuard; + if (!mpImpl) + return; mpImpl->getKeyListeners().removeInterface( rxListener ); } @@ -1076,6 +1083,8 @@ void VCLXWindow::addMouseListener( const css::uno::Reference< css::awt::XMouseLi void VCLXWindow::removeMouseListener( const css::uno::Reference< css::awt::XMouseListener >& rxListener ) { SolarMutexGuard aGuard; + if (!mpImpl) + return; mpImpl->getMouseListeners().removeInterface( rxListener ); } @@ -1088,6 +1097,8 @@ void VCLXWindow::addMouseMotionListener( const css::uno::Reference< css::awt::XM void VCLXWindow::removeMouseMotionListener( const css::uno::Reference< css::awt::XMouseMotionListener >& rxListener ) { SolarMutexGuard aGuard; + if (!mpImpl) + return; mpImpl->getMouseMotionListeners().removeInterface( rxListener ); } @@ -1100,6 +1111,8 @@ void VCLXWindow::addPaintListener( const css::uno::Reference< css::awt::XPaintLi void VCLXWindow::removePaintListener( const css::uno::Reference< css::awt::XPaintListener >& rxListener ) { SolarMutexGuard aGuard; + if (!mpImpl) + return; mpImpl->getPaintListeners().removeInterface( rxListener ); } @@ -2298,6 +2311,9 @@ void SAL_CALL VCLXWindow::disposing( const css::lang::EventObject& _rSource ) { SolarMutexGuard aGuard; + if (!mpImpl) + return; + // check if it comes from our AccessibleContext uno::Reference< uno::XInterface > aAC( mpImpl->mxAccessibleContext, uno::UNO_QUERY ); uno::Reference< uno::XInterface > xSource( _rSource.Source, uno::UNO_QUERY ); diff --git a/vcl/source/window/window.cxx b/vcl/source/window/window.cxx index 44f5438e3864..c0a9bf8b141b 100644 --- a/vcl/source/window/window.cxx +++ b/vcl/source/window/window.cxx @@ -547,6 +547,9 @@ void Window::dispose() mpWindowImpl->mpFrameData = nullptr; } + if (mpWindowImpl->mxWindowPeer) + mpWindowImpl->mxWindowPeer->dispose(); + // should be the last statements mpWindowImpl.reset(); @@ -3104,11 +3107,15 @@ const OUString& Window::GetHelpText() const void Window::SetWindowPeer( Reference< css::awt::XWindowPeer > const & xPeer, VCLXWindow* pVCLXWindow ) { - if (!mpWindowImpl) + if (!mpWindowImpl || mpWindowImpl->mbInDispose) return; // be safe against re-entrance: first clear the old ref, then assign the new one - mpWindowImpl->mxWindowPeer.clear(); + if (mpWindowImpl->mxWindowPeer) + { + mpWindowImpl->mxWindowPeer->dispose(); + mpWindowImpl->mxWindowPeer.clear(); + } mpWindowImpl->mxWindowPeer = xPeer; mpWindowImpl->mpVCLXWindow = pVCLXWindow; diff --git a/vcl/unx/gtk3/gtkframe.cxx b/vcl/unx/gtk3/gtkframe.cxx index c40a6b75b0e1..10fa515554f4 100644 --- a/vcl/unx/gtk3/gtkframe.cxx +++ b/vcl/unx/gtk3/gtkframe.cxx @@ -692,6 +692,13 @@ GtkSalFrame::~GtkSalFrame() while (m_nGrabLevel) removeGrabLevel(); + { + SolarMutexGuard aGuard; + + if(m_nWatcherId) + g_bus_unwatch_name(m_nWatcherId); + } + GtkWidget *pEventWidget = getMouseEventWidget(); for (auto handler_id : m_aMouseSignalIds) g_signal_handler_disconnect(G_OBJECT(pEventWidget), handler_id); @@ -706,9 +713,6 @@ GtkSalFrame::~GtkSalFrame() { SolarMutexGuard aGuard; - if(m_nWatcherId) - g_bus_unwatch_name(m_nWatcherId); - if( m_pWindow ) { g_object_set_data( G_OBJECT( m_pWindow ), "SalFrame", nullptr ); -- cgit v1.2.3