tdf#64711 pointer deleted out from under std::shared_ptr

the "end" call will close the shell, which is deleted directly so the local shared_ptr remains
pointed to something which is already deleted.

So:
a) Have activate return true/false to indicate the failure and require the caller to call
'end' in response to failure.
b) A bunch of stuff in the call-stack expects the ViewShell not to get deleted while they are
calling it, so additionally launch that 'end' to happen in the next event loop.

==2838867== Invalid read of size 8
==2838867==    at 0x32F4B83C: sd::ViewShellBase::GetDocShell() const (ViewShellBase.hxx:97)
==2838867==    by 0x335BBCFC: sd::ViewShell::GetDocSh() const (viewshel.cxx:1389)
==2838867==    by 0x3357CAC1: sd::PresentationViewShell::~PresentationViewShell() (presvish.cxx:73)
==2838867==    by 0x330AA34B: void __gnu_cxx::new_allocator<sd::PresentationViewShell>::destroy<sd::PresentationViewShell>(sd::PresentationViewShell*) (new_allocator.h:156)
==2838867==    by 0x330AA2DF: void std::allocator_traits<std::allocator<sd::PresentationViewShell> >::destroy<sd::PresentationViewShell>(std::allocator<sd::PresentationViewShell>&, sd::PresentationViewShell*) (alloc_traits.h:531)
==2838867==    by 0x330AA0BE: std::_Sp_counted_ptr_inplace<sd::PresentationViewShell, std::allocator<sd::PresentationViewShell>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:560)
==2838867==    by 0x32D10D33: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:158)
==2838867==    by 0x32D10C79: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (shared_ptr_base.h:733)
==2838867==    by 0x330A03BD: std::__shared_ptr<sd::PresentationViewShell, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1183)
==2838867==    by 0x3309F847: std::shared_ptr<sd::PresentationViewShell>::~shared_ptr() (shared_ptr.h:121)
==2838867==    by 0x3320E1E8: sd::SlideShow::activate(sd::ViewShellBase&) (slideshow.cxx:999)
==2838867==    by 0x3357CF33: sd::PresentationViewShell::Activate(bool) (presvish.cxx:118)

==2838867==  Address 0x2fb5f320 is 256 bytes inside a block of size 272 free'd
==2838867==    at 0x483BEDD: operator delete(void*) (vg_replace_malloc.c:584)
==2838867==    by 0x33466537: sd::PresentationViewShellBase::~PresentationViewShellBase() (PresentationViewShellBase.cxx:82)
==2838867==    by 0x823076C: SfxViewFrame::ReleaseObjectShell_Impl() (viewfrm.cxx:1113)
==2838867==    by 0x8234B1C: SfxViewFrame::~SfxViewFrame() (viewfrm.cxx:1652)
==2838867==    by 0x8234FEB: SfxViewFrame::~SfxViewFrame() (viewfrm.cxx:1646)
==2838867==    by 0x8230D6C: SfxViewFrame::Close() (viewfrm.cxx:1165)
==2838867==    by 0x81E4217: SfxFrame::DoClose_Impl() (frame.cxx:142)
==2838867==    by 0x821709A: SfxBaseController::dispose() (sfxbasecontroller.cxx:982)
==2838867==    by 0x3337A59F: sd::DrawController::dispose() (DrawController.cxx:164)
==2838867==    by 0x6F35CC0: (anonymous namespace)::XFrameImpl::setComponent(com::sun::star::uno::Reference<com::sun::star::awt::XWindow> const&, com::sun::star::uno::Reference<com::sun::star::frame::XController> const&) (frame.cxx:1485)
==2838867==    by 0x6F3834E: (anonymous namespace)::XFrameImpl::close(unsigned char) (frame.cxx:1692)
==2838867==    by 0x81E3CFA: SfxFrame::DoClose() (frame.cxx:108)
==2838867==    by 0x823802C: SfxViewFrame::DoClose() (viewfrm.cxx:2525)
==2838867==    by 0x3320B971: sd::SlideShow::end() (slideshow.cxx:696)
==2838867==    by 0x3320E1C2: sd::SlideShow::activate(sd::ViewShellBase&) (slideshow.cxx:995)
==2838867==    by 0x3357CF33: sd::PresentationViewShell::Activate(bool) (presvish.cxx:118)

Change-Id: Ida91228b7584491c9a5dc9c0b3f76ce63218a92a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103326
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@cib.de>

5 files changed, 45 insertions, 17 deletions

diff --git a/sd/source/ui/inc/PresentationViewShell.hxx b/sd/source/ui/inc/PresentationViewShell.hxx
index c0020685c02e..7adbc6631461 100644
--- a/sd/source/ui/inc/PresentationViewShell.hxx
+++ b/sd/source/ui/inc/PresentationViewShell.hxx
@@ -58,9 +58,12 @@ protected:
 
 private:
     ::tools::Rectangle       maOldVisArea;
+    ImplSVEvent* mnAbortSlideShowEvent;
 
     virtual void Activate (bool bIsMDIActivate) override;
     virtual void Paint (const ::tools::Rectangle& rRect, ::sd::Window* pWin) override;
+
+    DECL_LINK(AbortSlideShowHdl, void*, void);
 };
 
 } // end of namespace sd
diff --git a/sd/source/ui/inc/slideshow.hxx b/sd/source/ui/inc/slideshow.hxx
index 90a45ac826ab..9b6087a5d311 100644
--- a/sd/source/ui/inc/slideshow.hxx
+++ b/sd/source/ui/inc/slideshow.hxx
@@ -157,7 +157,8 @@ public:
 
     // events
     void resize( const Size &rSize );
-    void activate(ViewShellBase& rBase);
+    // return false if the activate failed. callers should call end in response to failre
+    bool activate(ViewShellBase& rBase);
     void deactivate();
     void paint();
 
diff --git a/sd/source/ui/slideshow/slideshow.cxx b/sd/source/ui/slideshow/slideshow.cxx
index 74bbe395d92d..d253bce66b4a 100644
--- a/sd/source/ui/slideshow/slideshow.cxx
+++ b/sd/source/ui/slideshow/slideshow.cxx
@@ -972,7 +972,7 @@ void SlideShow::resize( const Size &rSize )
         mxController->resize( rSize );
 }
 
-void SlideShow::activate( ViewShellBase& rBase )
+bool SlideShow::activate( ViewShellBase& rBase )
 {
     if( (mpFullScreenViewShellBase == &rBase) && !mxController.is() )
     {
@@ -984,23 +984,19 @@ void SlideShow::activate( ViewShellBase& rBase )
 
             CreateController( pShell.get(), pShell->GetView(), rBase.GetViewWindow() );
 
-            if( mxController->startShow(mxCurrentSettings.get()) )
-            {
-                pShell->Resize();
-                // Defer the sd::ShowWindow's GrabFocus to here. so that the accessible event can be fired correctly.
-                pShell->GetActiveWindow()->GrabFocus();
-            }
-            else
-            {
-                end();
-                return;
-            }
+            if (!mxController->startShow(mxCurrentSettings.get()))
+                return false;
+
+            pShell->Resize();
+            // Defer the sd::ShowWindow's GrabFocus to here. so that the accessible event can be fired correctly.
+            pShell->GetActiveWindow()->GrabFocus();
         }
     }
 
     if( mxController.is() )
         mxController->activate();
 
+    return true;
 }
 
 void SlideShow::deactivate()
diff --git a/sd/source/ui/view/presvish.cxx b/sd/source/ui/view/presvish.cxx
index e324b5803f7b..34a789f4dd18 100644
--- a/sd/source/ui/view/presvish.cxx
+++ b/sd/source/ui/view/presvish.cxx
@@ -61,7 +61,8 @@ void PresentationViewShell::InitInterface_Impl()
 
 
 PresentationViewShell::PresentationViewShell( ViewShellBase& rViewShellBase, vcl::Window* pParentWindow, FrameView* pFrameView)
-: DrawViewShell( rViewShellBase, pParentWindow, PageKind::Standard, pFrameView)
+    : DrawViewShell(rViewShellBase, pParentWindow, PageKind::Standard, pFrameView)
+    , mnAbortSlideShowEvent(nullptr)
 {
     if( GetDocSh() && GetDocSh()->GetCreateMode() == SfxObjectCreateMode::EMBEDDED )
         maOldVisArea = GetDocSh()->GetVisArea( ASPECT_CONTENT );
@@ -70,6 +71,9 @@ PresentationViewShell::PresentationViewShell( ViewShellBase& rViewShellBase, vcl
 
 PresentationViewShell::~PresentationViewShell()
 {
+    if (mnAbortSlideShowEvent)
+        Application::RemoveUserEvent(mnAbortSlideShowEvent);
+
     if( GetDocSh() && GetDocSh()->GetCreateMode() == SfxObjectCreateMode::EMBEDDED && !maOldVisArea.IsEmpty() )
         GetDocSh()->SetVisArea( maOldVisArea );
 }
@@ -102,6 +106,14 @@ VclPtr<SvxRuler> PresentationViewShell::CreateVRuler(::sd::Window*)
     return nullptr;
 }
 
+IMPL_LINK_NOARG(PresentationViewShell, AbortSlideShowHdl, void*, void)
+{
+    mnAbortSlideShowEvent = nullptr;
+    rtl::Reference<SlideShow> xSlideShow(SlideShow::GetSlideShow(GetViewShellBase()));
+    if (xSlideShow.is())
+        xSlideShow->end();
+}
+
 void PresentationViewShell::Activate( bool bIsMDIActivate )
 {
     DrawViewShell::Activate( bIsMDIActivate );
@@ -115,7 +127,20 @@ void PresentationViewShell::Activate( bool bIsMDIActivate )
 
         rtl::Reference< SlideShow > xSlideShow( SlideShow::GetSlideShow( GetViewShellBase() ) );
         if( xSlideShow.is() )
-            xSlideShow->activate(GetViewShellBase());
+        {
+            bool bSuccess = xSlideShow->activate(GetViewShellBase());
+            if (!bSuccess)
+            {
+                /* tdf#64711 PresentationViewShell is deleted by 'end' due to end closing
+                   the object shell. So if we call xSlideShow->end during Activate there are
+                   a lot of places in the call stack of Activate which understandable don't
+                   expect this ViewShell to be deleted during use. Defer to the next event
+                   loop the abort of the slideshow
+                */
+                if (!mnAbortSlideShowEvent)
+                    mnAbortSlideShowEvent = Application::PostUserEvent(LINK(this, PresentationViewShell, AbortSlideShowHdl));
+            }
+        }
 
         if( HasCurrentFunction() )
             GetCurrentFunction()->Activate();
diff --git a/sd/source/ui/view/viewshel.cxx b/sd/source/ui/view/viewshel.cxx
index ca99e3f7a214..cd7534a89893 100644
--- a/sd/source/ui/view/viewshel.cxx
+++ b/sd/source/ui/view/viewshel.cxx
@@ -320,8 +320,11 @@ void ViewShell::Activate(bool bIsMDIActivate)
         rBindings.Invalidate( SID_3D_STATE, true );
 
         rtl::Reference< SlideShow > xSlideShow( SlideShow::GetSlideShow( GetViewShellBase() ) );
-        if(xSlideShow.is() && xSlideShow->isRunning() )
-            xSlideShow->activate(GetViewShellBase());
+        if (xSlideShow.is() && xSlideShow->isRunning())
+        {
+            bool bSuccess = xSlideShow->activate(GetViewShellBase());
+            assert(bSuccess && "can only return false with a PresentationViewShell"); (void)bSuccess;
+        }
 
         if(HasCurrentFunction())
             GetCurrentFunction()->Activate();