fdo#72695: avoid double-free race condition for SwXDocumentIndex

Change-Id: I9264ea023ee12b24561e86d893b1f7abb2765621

5 files changed, 39 insertions, 24 deletions

diff --git a/sw/source/core/inc/unoidx.hxx b/sw/source/core/inc/unoidx.hxx
index a877774b0470..246bd9c45866 100644
--- a/sw/source/core/inc/unoidx.hxx
+++ b/sw/source/core/inc/unoidx.hxx
@@ -66,14 +66,15 @@ private:
 
     SwXDocumentIndex(SwTOXBaseSection const&, SwDoc &);
 
-public:
-
     /// descriptor
     SwXDocumentIndex(const TOXTypes eToxType, SwDoc& rDoc);
 
+public:
+
     static ::com::sun::star::uno::Reference<
             ::com::sun::star::text::XDocumentIndex>
-        CreateXDocumentIndex(SwDoc & rDoc, SwTOXBaseSection const& rSection);
+        CreateXDocumentIndex(SwDoc & rDoc, SwTOXBaseSection const* pSection,
+                TOXTypes eTypes = TOX_INDEX);
 
     // MetadatableMixin
     virtual ::sfx2::Metadatable* GetCoreObject() SAL_OVERRIDE;
diff --git a/sw/source/core/unocore/unocoll.cxx b/sw/source/core/unocore/unocoll.cxx
index 7dc282fc5065..9e0ce2ae8d48 100644
--- a/sw/source/core/unocore/unocoll.cxx
+++ b/sw/source/core/unocore/unocoll.cxx
@@ -659,7 +659,7 @@ uno::Reference< uno::XInterface >   SwXServiceProvider::MakeInstance(sal_uInt16
             {
                 eType = TOX_TABLES;
             }
-            xRet =  (cppu::OWeakObject*)new SwXDocumentIndex(eType, *pDoc);
+            xRet = SwXDocumentIndex::CreateXDocumentIndex(*pDoc, 0, eType);
         }
         break;
         case SW_SERVICE_INDEX_HEADER_SECTION :
diff --git a/sw/source/core/unocore/unocrsrhelper.cxx b/sw/source/core/unocore/unocrsrhelper.cxx
index faa62ae26e02..ef7e417fad7a 100644
--- a/sw/source/core/unocore/unocrsrhelper.cxx
+++ b/sw/source/core/unocore/unocrsrhelper.cxx
@@ -501,7 +501,7 @@ bool getCrsrPropertyValue(const SfxItemPropertySimpleEntry& rEntry
                 {
                     const uno::Reference< text::XDocumentIndex > xRef =
                         SwXDocumentIndex::CreateXDocumentIndex(*rPam.GetDoc(),
-                            *static_cast<SwTOXBaseSection const*>(pBase));
+                            static_cast<SwTOXBaseSection const*>(pBase));
                     (*pAny) <<= xRef;
                 }
             }
diff --git a/sw/source/core/unocore/unoidx.cxx b/sw/source/core/unocore/unoidx.cxx
index ef76573b6644..550f9f8a30e8 100644
--- a/sw/source/core/unocore/unoidx.cxx
+++ b/sw/source/core/unocore/unoidx.cxx
@@ -321,7 +321,7 @@ private:
     ::osl::Mutex m_Mutex; // just for OInterfaceContainerHelper
 
 public:
-    SwXDocumentIndex &          m_rThis;
+    uno::WeakReference<uno::XInterface> m_wThis;
     ::cppu::OMultiTypeInterfaceContainerHelper m_Listeners;
     SfxItemPropertySet const&   m_rPropSet;
     const TOXTypes              m_eTOXType;
@@ -331,12 +331,10 @@ public:
     uno::WeakReference<container::XIndexReplace> m_wStyleAccess;
     uno::WeakReference<container::XIndexReplace> m_wTokenAccess;
 
-    Impl(   SwXDocumentIndex & rThis,
-            SwDoc & rDoc,
+    Impl(   SwDoc & rDoc,
             const TOXTypes eType,
             SwTOXBaseSection const*const pBaseSection)
         : SwClient((pBaseSection) ? pBaseSection->GetFmt() : 0)
-        , m_rThis(rThis)
         , m_Listeners(m_Mutex)
         , m_rPropSet(
             *aSwMapProvider.GetPropertySet(lcl_TypeToPropertyMap_Index(eType)))
@@ -386,23 +384,29 @@ protected:
 void SwXDocumentIndex::Impl::Modify(const SfxPoolItem *pOld, const SfxPoolItem *pNew)
 {
     ClientModify(this, pOld, pNew);
-
-    if (!GetRegisteredIn())
+    if (GetRegisteredIn())
     {
-        lang::EventObject const ev(static_cast< ::cppu::OWeakObject&>(m_rThis));
-        m_Listeners.disposeAndClear(ev);
+        return; // core object still alive
+    }
+
+    uno::Reference<uno::XInterface> const xThis(m_wThis);
+    if (!xThis.is())
+    {   // fdo#72695: if UNO object is already dead, don't revive it with event
+        return;
     }
+    lang::EventObject const ev(xThis);
+    m_Listeners.disposeAndClear(ev);
 }
 
 SwXDocumentIndex::SwXDocumentIndex(
         SwTOXBaseSection const& rBaseSection, SwDoc & rDoc)
-    : m_pImpl( new SwXDocumentIndex::Impl( *this,
+    : m_pImpl( new SwXDocumentIndex::Impl(
                 rDoc, rBaseSection.SwTOXBase::GetType(), & rBaseSection) )
 {
 }
 
 SwXDocumentIndex::SwXDocumentIndex(const TOXTypes eType, SwDoc& rDoc)
-    : m_pImpl( new SwXDocumentIndex::Impl( *this, rDoc, eType, 0) )
+    : m_pImpl( new SwXDocumentIndex::Impl(rDoc, eType, 0) )
 {
 }
 
@@ -412,18 +416,28 @@ SwXDocumentIndex::~SwXDocumentIndex()
 
 uno::Reference<text::XDocumentIndex>
 SwXDocumentIndex::CreateXDocumentIndex(
-        SwDoc & rDoc, SwTOXBaseSection const& rSection)
+        SwDoc & rDoc, SwTOXBaseSection const* pSection, TOXTypes const eTypes)
 {
     // re-use existing SwXDocumentIndex
     // #i105557#: do not iterate over the registered clients: race condition
-    SwSectionFmt *const pFmt = rSection.GetFmt();
-    uno::Reference<text::XDocumentIndex> xIndex(pFmt->GetXObject(),
-            uno::UNO_QUERY);
+    uno::Reference<text::XDocumentIndex> xIndex;
+    if (pSection)
+    {
+        SwSectionFmt *const pFmt = pSection->GetFmt();
+        xIndex.set(pFmt->GetXObject(), uno::UNO_QUERY);
+    }
     if (!xIndex.is())
     {
-        SwXDocumentIndex *const pIndex(new SwXDocumentIndex(rSection, rDoc));
+        SwXDocumentIndex *const pIndex((pSection)
+                ? new SwXDocumentIndex(*pSection, rDoc)
+                : new SwXDocumentIndex(eTypes, rDoc));
         xIndex.set(pIndex);
-        pFmt->SetXObject(uno::Reference<uno::XInterface>(xIndex));
+        if (pSection)
+        {
+            pSection->GetFmt()->SetXObject(xIndex);
+        }
+        // need a permanent Reference to initialize m_wThis
+        pIndex->m_pImpl->m_wThis = xIndex;
     }
     return xIndex;
 }
@@ -2498,7 +2512,7 @@ throw (lang::IndexOutOfBoundsException, lang::WrappedTargetException,
         {
            const uno::Reference< text::XDocumentIndex > xTmp =
                SwXDocumentIndex::CreateXDocumentIndex(
-                   *GetDoc(), static_cast<SwTOXBaseSection const&>(*pSect));
+                   *GetDoc(), static_cast<SwTOXBaseSection const*>(pSect));
            uno::Any aRet;
            aRet <<= xTmp;
            return aRet;
@@ -2529,7 +2543,7 @@ throw (container::NoSuchElementException, lang::WrappedTargetException,
         {
            const uno::Reference< text::XDocumentIndex > xTmp =
                SwXDocumentIndex::CreateXDocumentIndex(
-                   *GetDoc(), static_cast<SwTOXBaseSection const&>(*pSect));
+                   *GetDoc(), static_cast<SwTOXBaseSection const*>(pSect));
            uno::Any aRet;
            aRet <<= xTmp;
            return aRet;
diff --git a/sw/source/core/unocore/unosect.cxx b/sw/source/core/unocore/unosect.cxx
index a47875bfe875..f88fee01ee41 100644
--- a/sw/source/core/unocore/unosect.cxx
+++ b/sw/source/core/unocore/unosect.cxx
@@ -1105,7 +1105,7 @@ throw (beans::UnknownPropertyException, lang::WrappedTargetException,
                     // convert section to TOXBase and get SwXDocumentIndex
                     const uno::Reference<text::XDocumentIndex> xIndex =
                         SwXDocumentIndex::CreateXDocumentIndex(
-                            *pTOXBaseSect->GetFmt()->GetDoc(), *pTOXBaseSect);
+                            *pTOXBaseSect->GetFmt()->GetDoc(), pTOXBaseSect);
                     pRet[nProperty] <<= xIndex;
                 }
                 // else: no enclosing index found -> empty return value