fdo#72695: avoid double-free race condition for SwXMeta/SwXMetaField

Change-Id: Ic5754dc479d7d55ce4e0f2713c5545db75a300c2

3 files changed, 38 insertions, 11 deletions

diff --git a/sw/source/core/inc/unometa.hxx b/sw/source/core/inc/unometa.hxx
index 0eee4e645c86..0948aad6d076 100644
--- a/sw/source/core/inc/unometa.hxx
+++ b/sw/source/core/inc/unometa.hxx
@@ -86,10 +86,10 @@ protected:
             xParentText,
         TextRangeList_t const*const pPortions);
 
-public:
-
     SwXMeta(SwDoc *const pDoc);
 
+public:
+
     SAL_WNODEPRECATED_DECLARATIONS_PUSH
     static ::com::sun::star::uno::Reference<
             ::com::sun::star::rdf::XMetadatable >
@@ -101,6 +101,9 @@ public:
                 ::std::auto_ptr<TextRangeList_t const>(0));
     SAL_WNODEPRECATED_DECLARATIONS_POP
 
+    static css::uno::Reference<css::rdf::XMetadatable>
+        CreateXMeta(SwDoc & rDoc, bool isField);
+
     /// init params with position of the attribute content (w/out CH_TXTATR)
     bool SetContentRange(
             SwTxtNode *& rpNode, sal_Int32 & rStart, sal_Int32 & rEnd) const;
@@ -260,10 +263,13 @@ private:
             xParentText,
         TextRangeList_t const*const pPortions);
 
-public:
+    friend css::uno::Reference<css::rdf::XMetadatable>
+        SwXMeta::CreateXMeta(SwDoc &, bool);
 
     SwXMetaField(SwDoc *const pDoc);
 
+public:
+
     // XServiceInfo
     virtual OUString SAL_CALL getImplementationName()
         throw (::com::sun::star::uno::RuntimeException, std::exception) SAL_OVERRIDE;
diff --git a/sw/source/core/unocore/unocoll.cxx b/sw/source/core/unocore/unocoll.cxx
index 47abfc7912e9..6b963f26022d 100644
--- a/sw/source/core/unocore/unocoll.cxx
+++ b/sw/source/core/unocore/unocoll.cxx
@@ -821,10 +821,10 @@ uno::Reference< uno::XInterface >   SwXServiceProvider::MakeInstance(sal_uInt16
                 xRet = (cppu::OWeakObject*) pDoc->getIDocumentChartDataProviderAccess().GetChartDataProvider( true /* create - if not yet available */ );
         break;
         case SW_SERVICE_TYPE_META:
-            xRet = static_cast< ::cppu::OWeakObject* >( new SwXMeta(pDoc) );
+            xRet = SwXMeta::CreateXMeta(*pDoc, false);
         break;
         case SW_SERVICE_FIELDTYPE_METAFIELD:
-            xRet = static_cast< ::cppu::OWeakObject* >(new SwXMetaField(pDoc));
+            xRet = SwXMeta::CreateXMeta(*pDoc, true);
         break;
         default:
             throw uno::RuntimeException();
diff --git a/sw/source/core/unocore/unorefmk.cxx b/sw/source/core/unocore/unorefmk.cxx
index 9a39778107fe..ad8aa19d800a 100644
--- a/sw/source/core/unocore/unorefmk.cxx
+++ b/sw/source/core/unocore/unorefmk.cxx
@@ -649,6 +649,7 @@ private:
     ::osl::Mutex m_Mutex; // just for OInterfaceContainerHelper
 
 public:
+    uno::WeakReference<uno::XInterface> m_wThis;
     ::cppu::OInterfaceContainerHelper m_EventListeners;
     SAL_WNODEPRECATED_DECLARATIONS_PUSH
     ::std::auto_ptr<const TextRangeList_t> m_pTextPortions;
@@ -694,14 +695,20 @@ void SwXMeta::Impl::Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew )
 
     ClientModify(this, pOld, pNew);
 
-    if (!GetRegisteredIn()) // removed => dispose
+    if (GetRegisteredIn())
     {
-        m_bIsDisposed = true;
-        lang::EventObject const ev(
-                static_cast< ::cppu::OWeakObject&>(m_Text.GetXMeta()));
-        m_EventListeners.disposeAndClear(ev);
-        m_Text.Invalidate();
+        return; // core object still alive
+    }
+
+    m_bIsDisposed = true;
+    m_Text.Invalidate();
+    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);
 }
 
 uno::Reference<text::XText> SwXMeta::GetParentText() const
@@ -725,6 +732,18 @@ SwXMeta::~SwXMeta()
 {
 }
 
+uno::Reference<rdf::XMetadatable>
+SwXMeta::CreateXMeta(SwDoc & rDoc, bool const isField)
+{
+    SwXMeta *const pXMeta((isField)
+            ? new SwXMetaField(& rDoc) : new SwXMeta(& rDoc));
+    // this is why the constructor is private: need to acquire pXMeta here
+    uno::Reference<rdf::XMetadatable> const xMeta(pXMeta);
+    // need a permanent Reference to initialize m_wThis
+    pXMeta->m_pImpl->m_wThis = xMeta;
+    return xMeta;
+}
+
 SAL_WNODEPRECATED_DECLARATIONS_PUSH
 uno::Reference<rdf::XMetadatable>
 SwXMeta::CreateXMeta(::sw::Meta & rMeta,
@@ -779,6 +798,8 @@ SwXMeta::CreateXMeta(::sw::Meta & rMeta,
     xMeta.set(pXMeta);
     // in order to initialize the weak pointer cache in the core object
     rMeta.SetXMeta(xMeta);
+    // need a permanent Reference to initialize m_wThis
+    pXMeta->m_pImpl->m_wThis = xMeta;
     return xMeta;
 }
 SAL_WNODEPRECATED_DECLARATIONS_POP