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

Change-Id: Id7832d8e65723ae30ad2b5ce95d145def53998f0

5 files changed, 31 insertions, 14 deletions

diff --git a/sw/source/core/inc/unofootnote.hxx b/sw/source/core/inc/unofootnote.hxx
index b5b6a2f0d91b..c9740960a540 100644
--- a/sw/source/core/inc/unofootnote.hxx
+++ b/sw/source/core/inc/unofootnote.hxx
@@ -67,13 +67,13 @@ protected:
     virtual ~SwXFootnote();
 
     SwXFootnote(SwDoc & rDoc, SwFmtFtn & rFmt);
+    SwXFootnote(const bool bEndnote);
 
 public:
 
-    SwXFootnote(const bool bEndnote);
-
     static css::uno::Reference<css::text::XFootnote>
-        CreateXFootnote(SwDoc & rDoc, SwFmtFtn & rFootnoteFmt);
+        CreateXFootnote(SwDoc & rDoc, SwFmtFtn * pFootnoteFmt,
+                bool isEndnote = false);
 
     // XInterface
     virtual ::com::sun::star::uno::Any SAL_CALL queryInterface(
diff --git a/sw/source/core/unocore/unocoll.cxx b/sw/source/core/unocore/unocoll.cxx
index 36fd4bae86d2..d08e8b907672 100644
--- a/sw/source/core/unocore/unocoll.cxx
+++ b/sw/source/core/unocore/unocoll.cxx
@@ -613,10 +613,10 @@ uno::Reference< uno::XInterface >   SwXServiceProvider::MakeInstance(sal_uInt16
         break;
 
         case  SW_SERVICE_TYPE_FOOTNOTE :
-            xRet =  (cppu::OWeakObject*)new SwXFootnote(false);
+            xRet = SwXFootnote::CreateXFootnote(*pDoc, 0, false);
         break;
         case  SW_SERVICE_TYPE_ENDNOTE  :
-            xRet =  (cppu::OWeakObject*)new SwXFootnote(true);
+            xRet = SwXFootnote::CreateXFootnote(*pDoc, 0, true);
         break;
         case  SW_SERVICE_CONTENT_INDEX_MARK :
         case  SW_SERVICE_USER_INDEX_MARK    :
@@ -1836,7 +1836,7 @@ uno::Any SwXFootnotes::getByIndex(sal_Int32 nIndex)
             if(nCount == nIndex)
             {
                 xRef = SwXFootnote::CreateXFootnote(*GetDoc(),
-                        const_cast<SwFmtFtn&>(rFtn));
+                        &const_cast<SwFmtFtn&>(rFtn));
                 aRet <<= xRef;
                 break;
             }
@@ -1865,7 +1865,7 @@ sal_Bool SwXFootnotes::hasElements(void) throw( uno::RuntimeException, std::exce
 
 Reference<XFootnote>    SwXFootnotes::GetObject( SwDoc& rDoc, const SwFmtFtn& rFmt )
 {
-    return SwXFootnote::CreateXFootnote(rDoc, const_cast<SwFmtFtn&>(rFmt));
+    return SwXFootnote::CreateXFootnote(rDoc, &const_cast<SwFmtFtn&>(rFmt));
 }
 
 OUString SwXReferenceMarks::getImplementationName(void) throw( RuntimeException, std::exception )
diff --git a/sw/source/core/unocore/unocrsrhelper.cxx b/sw/source/core/unocore/unocrsrhelper.cxx
index 6ea70b4a9dc8..6b511ae2fcca 100644
--- a/sw/source/core/unocore/unocrsrhelper.cxx
+++ b/sw/source/core/unocore/unocrsrhelper.cxx
@@ -608,7 +608,7 @@ bool getCrsrPropertyValue(const SfxItemPropertySimpleEntry& rEntry
                     {
                         const uno::Reference< text::XFootnote > xFootnote =
                             SwXFootnote::CreateXFootnote(*rPam.GetDoc(),
-                                    const_cast<SwFmtFtn&>(rFtn));
+                                    &const_cast<SwFmtFtn&>(rFtn));
                         *pAny <<= xFootnote;
                     }
                 }
diff --git a/sw/source/core/unocore/unoftn.cxx b/sw/source/core/unocore/unoftn.cxx
index ebafc93054f6..e3bd6b5fc4b4 100644
--- a/sw/source/core/unocore/unoftn.cxx
+++ b/sw/source/core/unocore/unoftn.cxx
@@ -50,6 +50,7 @@ private:
 public:
 
     SwXFootnote &               m_rThis;
+    uno::WeakReference<uno::XInterface> m_wThis;
     const bool                  m_bIsEndnote;
     ::cppu::OInterfaceContainerHelper m_EventListeners;
     bool                        m_bIsDescriptor;
@@ -96,7 +97,12 @@ void SwXFootnote::Impl::Invalidate()
     }
     m_pFmtFtn = 0;
     m_rThis.SetDoc(0);
-    lang::EventObject const ev(static_cast< ::cppu::OWeakObject&>(m_rThis));
+    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_EventListeners.disposeAndClear(ev);
 }
 
@@ -127,16 +133,27 @@ SwXFootnote::~SwXFootnote()
 }
 
 uno::Reference<text::XFootnote>
-SwXFootnote::CreateXFootnote(SwDoc & rDoc, SwFmtFtn & rFootnoteFmt)
+SwXFootnote::CreateXFootnote(SwDoc & rDoc, SwFmtFtn *const pFootnoteFmt,
+        bool const isEndnote)
 {
     // i#105557: do not iterate over the registered clients: race condition
     uno::Reference<text::XFootnote> xNote;
-    xNote = rFootnoteFmt.GetXFootnote();
+    if (pFootnoteFmt)
+    {
+        xNote = pFootnoteFmt->GetXFootnote();
+    }
     if (!xNote.is())
     {
-        SwXFootnote *const pNote(new SwXFootnote(rDoc, rFootnoteFmt));
+        SwXFootnote *const pNote((pFootnoteFmt)
+                ? new SwXFootnote(rDoc, *pFootnoteFmt)
+                : new SwXFootnote(isEndnote));
         xNote.set(pNote);
-        rFootnoteFmt.SetXFootnote(xNote);
+        if (pFootnoteFmt)
+        {
+            pFootnoteFmt->SetXFootnote(xNote);
+        }
+        // need a permanent Reference to initialize m_wThis
+        pNote->m_pImpl->m_wThis = xNote;
     }
     return xNote;
 }
diff --git a/sw/source/core/unocore/unoobj2.cxx b/sw/source/core/unocore/unoobj2.cxx
index a63fc6a13006..8ec6e7174c0a 100644
--- a/sw/source/core/unocore/unoobj2.cxx
+++ b/sw/source/core/unocore/unoobj2.cxx
@@ -1239,7 +1239,7 @@ CreateParentXText(SwDoc & rDoc, const SwPosition& rPos)
                                     FindSttNodeByType(SwFootnoteStartNode))
                 {
                     xParentText.set(SwXFootnote::CreateXFootnote(rDoc,
-                            const_cast<SwFmtFtn&>(rFtn)), uno::UNO_QUERY);
+                            &const_cast<SwFmtFtn&>(rFtn)), uno::UNO_QUERY);
                     break;
                 }
             }