tdf#142255 Crash when trying to insert a shape

regression from
commit 5aa60be574ece81b27c8f63e6e809871c694dba0
fix leak in VCLXWindow
it's too dangerous to dump the Impl during dispose(), because if I do,
the Impl is removed while we are iterating over listeners.

So just be check mbIsDisposing more heavily to prevent new references to
the VCLXWindow being added after it is disposed.

Change-Id: Ibb00a289440d24f531072ef5809613d5e8a0a98f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115598
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

3 files changed, 46 insertions, 26 deletions

diff --git a/toolkit/source/awt/vclxcontainer.cxx b/toolkit/source/awt/vclxcontainer.cxx
index 932c31b38234..e070c3239ab7 100644
--- a/toolkit/source/awt/vclxcontainer.cxx
+++ b/toolkit/source/awt/vclxcontainer.cxx
@@ -73,14 +73,16 @@ void VCLXContainer::addVclContainerListener( const css::uno::Reference< css::awt
 {
     SolarMutexGuard aGuard;
 
-    GetContainerListeners().addInterface( rxListener );
+    if (!IsDisposed())
+        GetContainerListeners().addInterface( rxListener );
 }
 
 void VCLXContainer::removeVclContainerListener( const css::uno::Reference< css::awt::XVclContainerListener >& rxListener )
 {
     SolarMutexGuard aGuard;
 
-    GetContainerListeners().removeInterface( rxListener );
+    if (!IsDisposed())
+        GetContainerListeners().removeInterface( rxListener );
 }
 
 css::uno::Sequence< css::uno::Reference< css::awt::XWindow > > VCLXContainer::getWindows(  )
diff --git a/toolkit/source/awt/vclxtopwindow.cxx b/toolkit/source/awt/vclxtopwindow.cxx
index 3a986a0f6d9e..010659a06985 100644
--- a/toolkit/source/awt/vclxtopwindow.cxx
+++ b/toolkit/source/awt/vclxtopwindow.cxx
@@ -92,14 +92,16 @@ void VCLXTopWindow::addTopWindowListener( const css::uno::Reference< css::awt::X
 {
     SolarMutexGuard aGuard;
 
-    GetTopWindowListeners().addInterface( rxListener );
+    if (!IsDisposed())
+        GetTopWindowListeners().addInterface( rxListener );
 }
 
 void VCLXTopWindow::removeTopWindowListener( const css::uno::Reference< css::awt::XTopWindowListener >& rxListener )
 {
     SolarMutexGuard aGuard;
 
-    GetTopWindowListeners().removeInterface( rxListener );
+    if (!IsDisposed())
+        GetTopWindowListeners().removeInterface( rxListener );
 }
 
 void VCLXTopWindow::toFront(  )
diff --git a/toolkit/source/awt/vclxwindow.cxx b/toolkit/source/awt/vclxwindow.cxx
index 616d93fe834d..2a90325ef9a3 100644
--- a/toolkit/source/awt/vclxwindow.cxx
+++ b/toolkit/source/awt/vclxwindow.cxx
@@ -328,7 +328,7 @@ VCLXWindow::VCLXWindow( bool _bWithDefaultProps )
 
 VCLXWindow::~VCLXWindow()
 {
-    assert(!mpImpl && "forgot to call dispose()");
+    assert(mpImpl->mbDisposing && "forgot to call dispose()");
 }
 
 
@@ -374,7 +374,7 @@ void VCLXWindow::resumeVclEventListening( )
 
 void VCLXWindow::notifyWindowRemoved( vcl::Window const & _rWindow )
 {
-    if ( mpImpl && mpImpl->getContainerListeners().getLength() )
+    if ( mpImpl->getContainerListeners().getLength() )
     {
         awt::VclContainerEvent aEvent;
         aEvent.Source = *this;
@@ -385,7 +385,7 @@ void VCLXWindow::notifyWindowRemoved( vcl::Window const & _rWindow )
 
 IMPL_LINK( VCLXWindow, WindowEventListener, VclWindowEvent&, rEvent, void )
 {
-    if ( mpImpl->mnListenerLockLevel )
+    if ( mpImpl->mbDisposing || mpImpl->mnListenerLockLevel )
         return;
 
     DBG_ASSERT( rEvent.GetWindow() && GetWindow(), "Window???" );
@@ -861,6 +861,8 @@ void VCLXWindow::ProcessWindowEvent( const VclWindowEvent& rVclWindowEvent )
 uno::Reference< accessibility::XAccessibleContext > VCLXWindow::CreateAccessibleContext()
 {
     SolarMutexGuard aGuard;
+    if (mpImpl->mbDisposing)
+        return nullptr;
     return getAccessibleFactory().createAccessibleContext( this );
 }
 
@@ -899,16 +901,13 @@ void VCLXWindow::dispose(  )
 {
     SolarMutexGuard aGuard;
 
-    if (!mpImpl)
-        return;
-
     if ( mpImpl->mbDisposing )
         return;
 
-    mpImpl->mxViewGraphics = nullptr;
-
     mpImpl->mbDisposing = true;
 
+    mpImpl->mxViewGraphics = nullptr;
+
     mpImpl->disposing();
 
     if ( auto pWindow = GetWindow() )
@@ -935,14 +934,13 @@ void VCLXWindow::dispose(  )
     {
         OSL_FAIL( "VCLXWindow::dispose: could not dispose the accessible context!" );
     }
-
-    mpImpl.reset();
+    mpImpl->mxAccessibleContext.clear();
 }
 
 void VCLXWindow::addEventListener( const css::uno::Reference< css::lang::XEventListener >& rxListener )
 {
     SolarMutexGuard aGuard;
-    if (!mpImpl) // called during dispose by accessibility stuff
+    if (mpImpl->mbDisposing) // called during dispose by accessibility stuff
         return;
     mpImpl->getEventListeners().addInterface( rxListener );
 }
@@ -950,7 +948,7 @@ void VCLXWindow::addEventListener( const css::uno::Reference< css::lang::XEventL
 void VCLXWindow::removeEventListener( const css::uno::Reference< css::lang::XEventListener >& rxListener )
 {
     SolarMutexGuard aGuard;
-    if (!mpImpl)
+    if (mpImpl->mbDisposing)
         return;
     mpImpl->getEventListeners().removeInterface( rxListener );
 }
@@ -1022,6 +1020,8 @@ void VCLXWindow::setFocus(  )
 void VCLXWindow::addWindowListener( const css::uno::Reference< css::awt::XWindowListener >& rxListener )
 {
     SolarMutexGuard aGuard;
+    if (mpImpl->mbDisposing)
+        return;
 
     mpImpl->getWindowListeners().addInterface( rxListener );
 
@@ -1038,7 +1038,7 @@ void VCLXWindow::removeWindowListener( const css::uno::Reference< css::awt::XWin
 {
     SolarMutexGuard aGuard;
 
-    if (!mpImpl)
+    if (mpImpl->mbDisposing)
         return;
 
     Reference< XWindowListener2 > xListener2( rxListener, UNO_QUERY );
@@ -1051,13 +1051,15 @@ void VCLXWindow::removeWindowListener( const css::uno::Reference< css::awt::XWin
 void VCLXWindow::addFocusListener( const css::uno::Reference< css::awt::XFocusListener >& rxListener )
 {
     SolarMutexGuard aGuard;
+    if (mpImpl->mbDisposing)
+        return;
     mpImpl->getFocusListeners().addInterface( rxListener );
 }
 
 void VCLXWindow::removeFocusListener( const css::uno::Reference< css::awt::XFocusListener >& rxListener )
 {
     SolarMutexGuard aGuard;
-    if (!mpImpl)
+    if (mpImpl->mbDisposing)
         return;
     mpImpl->getFocusListeners().removeInterface( rxListener );
 }
@@ -1065,13 +1067,15 @@ void VCLXWindow::removeFocusListener( const css::uno::Reference< css::awt::XFocu
 void VCLXWindow::addKeyListener( const css::uno::Reference< css::awt::XKeyListener >& rxListener )
 {
     SolarMutexGuard aGuard;
+    if (mpImpl->mbDisposing)
+        return;
     mpImpl->getKeyListeners().addInterface( rxListener );
 }
 
 void VCLXWindow::removeKeyListener( const css::uno::Reference< css::awt::XKeyListener >& rxListener )
 {
     SolarMutexGuard aGuard;
-    if (!mpImpl)
+    if (mpImpl->mbDisposing)
         return;
     mpImpl->getKeyListeners().removeInterface( rxListener );
 }
@@ -1079,13 +1083,15 @@ void VCLXWindow::removeKeyListener( const css::uno::Reference< css::awt::XKeyLis
 void VCLXWindow::addMouseListener( const css::uno::Reference< css::awt::XMouseListener >& rxListener )
 {
     SolarMutexGuard aGuard;
+    if (mpImpl->mbDisposing)
+        return;
     mpImpl->getMouseListeners().addInterface( rxListener );
 }
 
 void VCLXWindow::removeMouseListener( const css::uno::Reference< css::awt::XMouseListener >& rxListener )
 {
     SolarMutexGuard aGuard;
-    if (!mpImpl)
+    if (mpImpl->mbDisposing)
         return;
     mpImpl->getMouseListeners().removeInterface( rxListener );
 }
@@ -1093,13 +1099,15 @@ void VCLXWindow::removeMouseListener( const css::uno::Reference< css::awt::XMous
 void VCLXWindow::addMouseMotionListener( const css::uno::Reference< css::awt::XMouseMotionListener >& rxListener )
 {
     SolarMutexGuard aGuard;
+    if (mpImpl->mbDisposing)
+        return;
     mpImpl->getMouseMotionListeners().addInterface( rxListener );
 }
 
 void VCLXWindow::removeMouseMotionListener( const css::uno::Reference< css::awt::XMouseMotionListener >& rxListener )
 {
     SolarMutexGuard aGuard;
-    if (!mpImpl)
+    if (mpImpl->mbDisposing)
         return;
     mpImpl->getMouseMotionListeners().removeInterface( rxListener );
 }
@@ -1107,13 +1115,15 @@ void VCLXWindow::removeMouseMotionListener( const css::uno::Reference< css::awt:
 void VCLXWindow::addPaintListener( const css::uno::Reference< css::awt::XPaintListener >& rxListener )
 {
     SolarMutexGuard aGuard;
+    if (mpImpl->mbDisposing)
+        return;
     mpImpl->getPaintListeners().addInterface( rxListener );
 }
 
 void VCLXWindow::removePaintListener( const css::uno::Reference< css::awt::XPaintListener >& rxListener )
 {
     SolarMutexGuard aGuard;
-    if (!mpImpl)
+    if (mpImpl->mbDisposing)
         return;
     mpImpl->getPaintListeners().removeInterface( rxListener );
 }
@@ -2313,7 +2323,7 @@ void SAL_CALL VCLXWindow::disposing( const css::lang::EventObject& _rSource )
 {
     SolarMutexGuard aGuard;
 
-    if (!mpImpl)
+    if (mpImpl->mbDisposing)
         return;
 
     // check if it comes from our AccessibleContext
@@ -2332,7 +2342,7 @@ css::uno::Reference< css::accessibility::XAccessibleContext > VCLXWindow::getAcc
     SolarMutexGuard aGuard;
 
     // already disposed
-    if( ! mpImpl )
+    if (mpImpl->mbDisposing)
         return uno::Reference< accessibility::XAccessibleContext >();
 
     if ( !mpImpl->mxAccessibleContext.is() && GetWindow() )
@@ -2355,7 +2365,7 @@ void SAL_CALL VCLXWindow::addDockableWindowListener( const css::uno::Reference<
 {
     SolarMutexGuard aGuard;
 
-    if ( xListener.is() )
+    if (!mpImpl->mbDisposing && xListener.is() )
         mpImpl->getDockableWindowListeners().addInterface( xListener );
 
 }
@@ -2364,7 +2374,8 @@ void SAL_CALL VCLXWindow::removeDockableWindowListener( const css::uno::Referenc
 {
     SolarMutexGuard aGuard;
 
-    mpImpl->getDockableWindowListeners().removeInterface( xListener );
+    if (!mpImpl->mbDisposing)
+        mpImpl->getDockableWindowListeners().removeInterface( xListener );
 }
 
 void SAL_CALL VCLXWindow::enableDocking( sal_Bool bEnable )
@@ -2529,4 +2540,9 @@ Reference< XStyleSettings > SAL_CALL VCLXWindow::getStyleSettings()
     return mpImpl->getStyleSettings();
 }
 
+bool VCLXWindow::IsDisposed()
+{
+    return mpImpl->mbDisposing;
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */