use unique_ptr in SfxAppData_Impl

and simplify the destruction - we only ever called ReleaseInterface
on destruction, so no need to bother with that, and risk touching
pointers that might be nullptr

Specifically, the problem is this code
pImpl->pSlotPool.reset();
which triggered the path
~SfxSlotPool -> ~SfxInterface -> SfxGetpApp()->GetAppSlotPool_Impl()
which crashes because the pSlotPool pointer is null at that point

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

10 files changed, 63 insertions, 88 deletions

diff --git a/sfx2/source/appl/app.cxx b/sfx2/source/appl/app.cxx
index 6844f919c4b2..fbd45b3e49b3 100644
--- a/sfx2/source/appl/app.cxx
+++ b/sfx2/source/appl/app.cxx
@@ -235,7 +235,7 @@ void SfxApplication::ResetLastDir()
 
 SfxDispatcher* SfxApplication::GetDispatcher_Impl()
 {
-    return pImpl->pViewFrame? pImpl->pViewFrame->GetDispatcher(): pImpl->pAppDispat;
+    return pImpl->pViewFrame ? pImpl->pViewFrame->GetDispatcher() : pImpl->pAppDispat.get();
 }
 
 
diff --git a/sfx2/source/appl/appchild.cxx b/sfx2/source/appl/appchild.cxx
index 0aaf722ba700..9ee356566479 100644
--- a/sfx2/source/appl/appchild.cxx
+++ b/sfx2/source/appl/appchild.cxx
@@ -39,7 +39,7 @@ void SfxApplication::RegisterChildWindow_Impl( SfxModule *pMod, std::unique_ptr<
     }
 
     if (!pImpl->pFactArr)
-        pImpl->pFactArr = new SfxChildWinFactArr_Impl;
+        pImpl->pFactArr.reset(new SfxChildWinFactArr_Impl);
 
     for (size_t nFactory=0; nFactory<pImpl->pFactArr->size(); ++nFactory)
     {
@@ -83,7 +83,7 @@ void SfxApplication::RegisterChildWindowContext_Impl( SfxModule *pMod, sal_uInt1
         DBG_ASSERT( pImpl, "No AppData!" );
         DBG_ASSERT( pImpl->pFactArr, "No Factories!" );
 
-        pFactories = pImpl->pFactArr;
+        pFactories = pImpl->pFactArr.get();
         sal_uInt16 nCount = pFactories->size();
         for (sal_uInt16 nFactory=0; nFactory<nCount; ++nFactory)
         {
diff --git a/sfx2/source/appl/appdata.cxx b/sfx2/source/appl/appdata.cxx
index fdc3f0304df2..0039cdae17a5 100644
--- a/sfx2/source/appl/appdata.cxx
+++ b/sfx2/source/appl/appdata.cxx
@@ -21,12 +21,18 @@
 
 #include <appdata.hxx>
 #include <sfxpicklist.hxx>
+#include <sfx2/dispatch.hxx>
 #include <sfx2/doctempl.hxx>
+#include <sfx2/fcontnr.hxx>
 #include <sfx2/module.hxx>
+#include <sfx2/msgpool.hxx>
 #include <sfx2/sidebar/Theme.hxx>
 #include <sfx2/objsh.hxx>
-#include <unoctitm.hxx>
 #include <appbaslib.hxx>
+#include <childwinimpl.hxx>
+#include <ctrlfactoryimpl.hxx>
+#include <shellimpl.hxx>
+#include <unoctitm.hxx>
 #include <svl/svdde.hxx>
 
 #include <basic/basicmanagerrepository.hxx>
@@ -68,28 +74,13 @@ void SfxBasicManagerCreationListener::onBasicManagerCreated( const Reference< XM
 }
 
 SfxAppData_Impl::SfxAppData_Impl()
-    : pFactArr(nullptr)
-    , pMatcher( nullptr )
-    , m_pToolsErrorHdl(nullptr)
-    , m_pSoErrorHdl(nullptr)
-#if HAVE_FEATURE_SCRIPTING
-    , m_pSbxErrorHdl(nullptr)
-#endif
-    , pTemplates( nullptr )
-    , pPool(nullptr)
+    : pPool(nullptr)
     , pProgress(nullptr)
     , nDocModalMode(0)
     , nRescheduleLocks(0)
-    , pTbxCtrlFac(nullptr)
-    , pStbCtrlFac(nullptr)
-    , pViewFrames(nullptr)
-    , pViewShells(nullptr)
-    , pObjShells(nullptr)
     , pBasicManager( new SfxBasicManagerHolder )
     , pBasMgrListener( new SfxBasicManagerCreationListener( *this ) )
     , pViewFrame( nullptr )
-    , pSlotPool( nullptr )
-    , pAppDispat( nullptr )
     , bDowning( true )
     , bInQuit( false )
 
@@ -117,10 +108,10 @@ SfxAppData_Impl::~SfxAppData_Impl()
 SfxDocumentTemplates* SfxAppData_Impl::GetDocumentTemplates()
 {
     if ( !pTemplates )
-        pTemplates = new SfxDocumentTemplates;
+        pTemplates.reset(new SfxDocumentTemplates);
     else
         pTemplates->ReInitFromComponent();
-    return pTemplates;
+    return pTemplates.get();
 }
 
 void SfxAppData_Impl::OnApplicationBasicManagerCreated( BasicManager& _rBasicManager )
diff --git a/sfx2/source/appl/appinit.cxx b/sfx2/source/appl/appinit.cxx
index 3ebc6a83bf5d..fbecd9d66b61 100644
--- a/sfx2/source/appl/appinit.cxx
+++ b/sfx2/source/appl/appinit.cxx
@@ -190,14 +190,14 @@ void SfxApplication::Initialize_Impl()
     Help::EnableContextHelp();
     Help::EnableExtHelp();
 
-    pImpl->m_pToolsErrorHdl = new SfxErrorHandler(
-        RID_ERRHDL, ErrCodeArea::Io, ErrCodeArea::Vcl);
+    pImpl->m_pToolsErrorHdl.reset(new SfxErrorHandler(
+        RID_ERRHDL, ErrCodeArea::Io, ErrCodeArea::Vcl));
 
-    pImpl->m_pSoErrorHdl = new SfxErrorHandler(
-        RID_SO_ERROR_HANDLER, ErrCodeArea::So, ErrCodeArea::So, SvtResLocale());
+    pImpl->m_pSoErrorHdl.reset(new SfxErrorHandler(
+        RID_SO_ERROR_HANDLER, ErrCodeArea::So, ErrCodeArea::So, SvtResLocale()));
 #if HAVE_FEATURE_SCRIPTING
-    pImpl->m_pSbxErrorHdl = new SfxErrorHandler(
-        RID_BASIC_START, ErrCodeArea::Sbx, ErrCodeArea::Sbx, BasResLocale());
+    pImpl->m_pSbxErrorHdl.reset(new SfxErrorHandler(
+        RID_BASIC_START, ErrCodeArea::Sbx, ErrCodeArea::Sbx, BasResLocale()));
 #endif
 
     if (!utl::ConfigManager::IsFuzzing())
@@ -209,13 +209,13 @@ void SfxApplication::Initialize_Impl()
     }
 
     DBG_ASSERT( !pImpl->pAppDispat, "AppDispatcher already exists" );
-    pImpl->pAppDispat = new SfxDispatcher;
-    pImpl->pSlotPool = new SfxSlotPool;
-    pImpl->pTbxCtrlFac = new SfxTbxCtrlFactArr_Impl;
-    pImpl->pStbCtrlFac = new SfxStbCtrlFactArr_Impl;
-    pImpl->pViewFrames = new SfxViewFrameArr_Impl;
-    pImpl->pViewShells = new SfxViewShellArr_Impl;
-    pImpl->pObjShells = new SfxObjectShellArr_Impl;
+    pImpl->pAppDispat.reset(new SfxDispatcher);
+    pImpl->pSlotPool.reset(new SfxSlotPool);
+    pImpl->pTbxCtrlFac.reset(new SfxTbxCtrlFactArr_Impl);
+    pImpl->pStbCtrlFac.reset(new SfxStbCtrlFactArr_Impl);
+    pImpl->pViewFrames.reset(new SfxViewFrameArr_Impl);
+    pImpl->pViewShells.reset(new SfxViewShellArr_Impl);
+    pImpl->pObjShells.reset(new SfxObjectShellArr_Impl);
 
     Registrations_Impl();
 
diff --git a/sfx2/source/appl/appmain.cxx b/sfx2/source/appl/appmain.cxx
index 31295cd0f5ae..884c4fd4db39 100644
--- a/sfx2/source/appl/appmain.cxx
+++ b/sfx2/source/appl/appmain.cxx
@@ -28,9 +28,9 @@ SfxFilterMatcher& SfxApplication::GetFilterMatcher()
 {
     if( !pImpl->pMatcher )
     {
-        pImpl->pMatcher = new SfxFilterMatcher();
+        pImpl->pMatcher.reset(new SfxFilterMatcher());
         URIHelper::SetMaybeFileHdl( LINK(
-            pImpl->pMatcher, SfxFilterMatcher, MaybeFileHdl_Impl ) );
+            pImpl->pMatcher.get(), SfxFilterMatcher, MaybeFileHdl_Impl ) );
     }
     return *pImpl->pMatcher;
 }
diff --git a/sfx2/source/appl/appmisc.cxx b/sfx2/source/appl/appmisc.cxx
index ada8f86eed04..53613b8921d9 100644
--- a/sfx2/source/appl/appmisc.cxx
+++ b/sfx2/source/appl/appmisc.cxx
@@ -99,7 +99,7 @@ SfxModule* SfxApplication::GetModule_Impl()
 }
 
 bool  SfxApplication::IsDowning() const { return pImpl->bDowning; }
-SfxDispatcher* SfxApplication::GetAppDispatcher_Impl() { return pImpl->pAppDispat; }
+SfxDispatcher* SfxApplication::GetAppDispatcher_Impl() { return pImpl->pAppDispat.get(); }
 SfxSlotPool& SfxApplication::GetAppSlotPool_Impl() const { return *pImpl->pSlotPool; }
 
 static bool FileExists( const INetURLObject& rURL )
diff --git a/sfx2/source/appl/appquit.cxx b/sfx2/source/appl/appquit.cxx
index 7b17c538c14f..829cc7601770 100644
--- a/sfx2/source/appl/appquit.cxx
+++ b/sfx2/source/appl/appquit.cxx
@@ -54,7 +54,7 @@ void SfxApplication::Deinitialize()
 
     pImpl->bDowning = true; // due to Timer from DecAliveCount and QueryExit
 
-    DELETEZ( pImpl->pTemplates );
+    pImpl->pTemplates.reset();
 
     // By definition there shouldn't be any open view frames when we reach
     // this method. Therefore this call makes no sense and is the source of
@@ -82,19 +82,19 @@ void SfxApplication::Deinitialize()
     DBG_ASSERT( pImpl->pViewFrame == nullptr, "active foreign ViewFrame" );
 
     // free administration managers
-    DELETEZ(pImpl->pAppDispat);
+    pImpl->pAppDispat.reset();
 
     // from here no SvObjects have to exists
-    DELETEZ(pImpl->pMatcher);
+    pImpl->pMatcher.reset();
 
-    DELETEZ(pImpl->pSlotPool);
-    DELETEZ(pImpl->pFactArr);
+    pImpl->pSlotPool.reset();
+    pImpl->pFactArr.reset();
 
-    DELETEZ(pImpl->pTbxCtrlFac);
-    DELETEZ(pImpl->pStbCtrlFac);
-    DELETEZ(pImpl->pViewFrames);
-    DELETEZ(pImpl->pViewShells);
-    DELETEZ(pImpl->pObjShells);
+    pImpl->pTbxCtrlFac.reset();
+    pImpl->pStbCtrlFac.reset();
+    pImpl->pViewFrames.reset();
+    pImpl->pViewShells.reset();
+    pImpl->pObjShells.reset();
 
     //TODO/CLEANUP
     //ReleaseArgs could be used instead!
@@ -102,10 +102,10 @@ void SfxApplication::Deinitialize()
     NoChaos::ReleaseItemPool();
 
 #if HAVE_FEATURE_SCRIPTING
-    delete pImpl->m_pSbxErrorHdl;
+    pImpl->m_pSbxErrorHdl.reset();
 #endif
-    delete pImpl->m_pSoErrorHdl;
-    delete pImpl->m_pToolsErrorHdl;
+    pImpl->m_pSoErrorHdl.reset();
+    pImpl->m_pToolsErrorHdl.reset();
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sfx2/source/control/msgpool.cxx b/sfx2/source/control/msgpool.cxx
index 28a1c8ba885b..be4ebdaa67f9 100644
--- a/sfx2/source/control/msgpool.cxx
+++ b/sfx2/source/control/msgpool.cxx
@@ -150,16 +150,6 @@ const std::type_info* SfxSlotPool::GetSlotType( sal_uInt16 nId ) const
 }
 
 
-// unregisters the availability of the Interface of functions
-
-void SfxSlotPool::ReleaseInterface( SfxInterface& rInterface )
-{
-    // remove from the list of SfxInterface instances
-    auto i = std::find(_vInterfaces.begin(), _vInterfaces.end(), &rInterface);
-    if(i != _vInterfaces.end())
-        _vInterfaces.erase(i);
-}
-
 // get the first SfxMessage for a special Id (e.g. for getting check-mode)
 
 const SfxSlot* SfxSlotPool::GetSlot( sal_uInt16 nId ) const
diff --git a/sfx2/source/control/objface.cxx b/sfx2/source/control/objface.cxx
index d794d715a9c2..f995eb9184e6 100644
--- a/sfx2/source/control/objface.cxx
+++ b/sfx2/source/control/objface.cxx
@@ -185,20 +185,6 @@ void SfxInterface::SetSlotMap( SfxSlot& rSlotMap, sal_uInt16 nSlotCount )
 
 SfxInterface::~SfxInterface()
 {
-    SfxModule *pMod = pImplData->pModule;
-    bool bRegistered = pImplData->bRegistered;
-    assert( bRegistered );
-    if ( bRegistered )
-    {
-        if ( pMod )
-        {
-            // can return nullptr if we are called from the SfxSlotPool destructor
-            if (pMod->GetSlotPool())
-                pMod->GetSlotPool()->ReleaseInterface(*this);
-        }
-        else
-            SfxGetpApp()->GetAppSlotPool_Impl().ReleaseInterface(*this);
-    }
 }
 
 
diff --git a/sfx2/source/inc/appdata.hxx b/sfx2/source/inc/appdata.hxx
index 2e46ce22eb92..f36a25f176e6 100644
--- a/sfx2/source/inc/appdata.hxx
+++ b/sfx2/source/inc/appdata.hxx
@@ -73,19 +73,20 @@ public:
     std::unique_ptr<DdeService>              pDdeService2;
 
     // single instance classes
-    SfxChildWinFactArr_Impl*            pFactArr;
+    std::unique_ptr<SfxChildWinFactArr_Impl>
+                                        pFactArr;
     std::vector<SfxFrame*>              vTopFrames;
 
     // application members
-    SfxFilterMatcher*                   pMatcher;
-    SfxErrorHandler *m_pToolsErrorHdl;
-    SfxErrorHandler *m_pSoErrorHdl;
+    std::unique_ptr<SfxFilterMatcher>   pMatcher;
+    std::unique_ptr<SfxErrorHandler>    m_pToolsErrorHdl;
+    std::unique_ptr<SfxErrorHandler>    m_pSoErrorHdl;
 #if HAVE_FEATURE_SCRIPTING
-    SfxErrorHandler *m_pSbxErrorHdl;
+    std::unique_ptr<SfxErrorHandler>    m_pSbxErrorHdl;
 #endif
     rtl::Reference<SfxStatusDispatcher> mxAppDispatch;
     std::unique_ptr<SfxPickList>        mxAppPickList;
-    SfxDocumentTemplates*               pTemplates;
+    std::unique_ptr<SfxDocumentTemplates> pTemplates;
 
     // global pointers
     SfxItemPool*                        pPool;
@@ -96,18 +97,25 @@ public:
     sal_uInt16                              nDocModalMode;              // counts documents in modal mode
     sal_uInt16                              nRescheduleLocks;
 
-    SfxTbxCtrlFactArr_Impl*     pTbxCtrlFac;
-    SfxStbCtrlFactArr_Impl*     pStbCtrlFac;
-    SfxViewFrameArr_Impl*       pViewFrames;
-    SfxViewShellArr_Impl*       pViewShells;
-    SfxObjectShellArr_Impl*     pObjShells;
+    std::unique_ptr<SfxTbxCtrlFactArr_Impl>
+                                pTbxCtrlFac;
+    std::unique_ptr<SfxStbCtrlFactArr_Impl>
+                                pStbCtrlFac;
+    std::unique_ptr<SfxViewFrameArr_Impl>
+                                pViewFrames;
+    std::unique_ptr<SfxViewShellArr_Impl>
+                                pViewShells;
+    std::unique_ptr<SfxObjectShellArr_Impl>
+                                pObjShells;
     std::unique_ptr<SfxBasicManagerHolder>
                                 pBasicManager;
     std::unique_ptr<SfxBasicManagerCreationListener>
                                 pBasMgrListener;
     SfxViewFrame*               pViewFrame;
-    SfxSlotPool*                pSlotPool;
-    SfxDispatcher*              pAppDispat;     // Dispatcher if no document
+    std::unique_ptr<SfxSlotPool>
+                                pSlotPool;
+    std::unique_ptr<SfxDispatcher>
+                                pAppDispat;     // Dispatcher if no document
     ::rtl::Reference<sfx2::sidebar::Theme> m_pSidebarTheme;
 
     bool                        bDowning:1;   // sal_True on Exit and afterwards