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

Change-Id: I6adfcb1bdd5e8e1525568a4bcd93a8e0e97359be

5 files changed, 58 insertions, 33 deletions

diff --git a/sw/source/core/inc/unofield.hxx b/sw/source/core/inc/unofield.hxx
index 441145ce57d4..0e30e97526c7 100644
--- a/sw/source/core/inc/unofield.hxx
+++ b/sw/source/core/inc/unofield.hxx
@@ -170,15 +170,16 @@ private:
 
     SwXTextField(const SwFmtFld& rFmt, SwDoc & rDoc);
 
-public:
     /// descriptor
     SwXTextField(sal_uInt16 nServiceId, SwDoc* pDoc=0);
 
+public:
     sal_uInt16 GetServiceId() const;
 
     /// @return an SwXTextField, either an already existing one or a new one
     static ::com::sun::star::uno::Reference< ::com::sun::star::text::XTextField>
-        CreateXTextField(SwDoc & rDoc, SwFmtFld const& rFmt);
+        CreateXTextField(SwDoc * pDoc, SwFmtFld const* pFmt,
+                sal_uInt16 nServiceId = 0xFFFF);
 
     static const ::com::sun::star::uno::Sequence< sal_Int8 > & getUnoTunnelId();
 
diff --git a/sw/source/core/unocore/unocoll.cxx b/sw/source/core/unocore/unocoll.cxx
index e2a458a056e5..e9dcf830bbe6 100644
--- a/sw/source/core/unocore/unocoll.cxx
+++ b/sw/source/core/unocore/unocoll.cxx
@@ -755,10 +755,11 @@ uno::Reference< uno::XInterface >   SwXServiceProvider::MakeInstance(sal_uInt16
         case SW_SERVICE_FIELDTYPE_COMBINED_CHARACTERS       :
         case SW_SERVICE_FIELDTYPE_DROPDOWN                  :
         case SW_SERVICE_FIELDTYPE_TABLE_FORMULA:
-            xRet = (cppu::OWeakObject*)new SwXTextField(nObjectType);
+            // NOTE: the sw.SwXAutoTextEntry unoapi test depends on pDoc = 0
+            xRet = SwXTextField::CreateXTextField(0, 0, nObjectType);
             break;
         case SW_SERVICE_FIELDTYPE_ANNOTATION:
-            xRet = (cppu::OWeakObject*)new SwXTextField(nObjectType, pDoc);
+            xRet = SwXTextField::CreateXTextField(pDoc, 0, nObjectType);
             break;
         case SW_SERVICE_FIELDMASTER_USER:
         case SW_SERVICE_FIELDMASTER_DDE:
diff --git a/sw/source/core/unocore/unocrsrhelper.cxx b/sw/source/core/unocore/unocrsrhelper.cxx
index cc5a0333860d..51dcd9ba39d3 100644
--- a/sw/source/core/unocore/unocrsrhelper.cxx
+++ b/sw/source/core/unocore/unocrsrhelper.cxx
@@ -522,8 +522,8 @@ bool getCrsrPropertyValue(const SfxItemPropertySimpleEntry& rEntry
                 if( pAny )
                 {
                     uno::Reference<text::XTextField> const xField(
-                        SwXTextField::CreateXTextField(*rPam.GetDoc(),
-                           pTxtAttr->GetFmtFld()));
+                        SwXTextField::CreateXTextField(rPam.GetDoc(),
+                           &pTxtAttr->GetFmtFld()));
                     *pAny <<= xField;
                 }
             }
diff --git a/sw/source/core/unocore/unofield.cxx b/sw/source/core/unocore/unofield.cxx
index 01d3a47320ae..353d0d4d4505 100644
--- a/sw/source/core/unocore/unofield.cxx
+++ b/sw/source/core/unocore/unofield.cxx
@@ -846,7 +846,7 @@ throw (beans::UnknownPropertyException, lang::WrappedTargetException,
             {
                 pFld = aFldArr[i];
                 uno::Reference<text::XTextField> const xField =
-                    SwXTextField::CreateXTextField(*m_pImpl->m_pDoc, *pFld);
+                    SwXTextField::CreateXTextField(m_pImpl->m_pDoc, pFld);
 
                 pRetSeq[i] = uno::Reference<text::XDependentTextField>(xField,
                         uno::UNO_QUERY);
@@ -1065,20 +1065,6 @@ OUString SwXFieldMaster::LocalizeFormula(
     return rFormula;
 }
 
-uno::Reference<text::XTextField>
-SwXTextField::CreateXTextField(SwDoc & rDoc, SwFmtFld const& rFmt)
-{
-    // re-use existing SwXTextField
-    uno::Reference<text::XTextField> xField(rFmt.GetXTextField());
-    if (!xField.is())
-    {
-        SwXTextField *const pField(new SwXTextField(rFmt, rDoc));
-        xField.set(pField);
-        const_cast<SwFmtFld &>(rFmt).SetXTextField(xField);
-    }
-    return xField;
-}
-
 struct SwFieldProperties_Impl
 {
     OUString    sPar1;
@@ -1131,9 +1117,9 @@ class SwXTextField::Impl
 {
 private:
     ::osl::Mutex m_Mutex; // just for OInterfaceContainerHelper
-    SwXTextField & m_rThis;
 
 public:
+    uno::WeakReference<uno::XInterface> m_wThis;
     ::cppu::OInterfaceContainerHelper m_EventListeners;
 
     SwFmtFld const*     m_pFmtFld;
@@ -1148,10 +1134,9 @@ public:
     OUString            m_sTypeName;
     boost::scoped_ptr<SwFieldProperties_Impl> m_pProps;
 
-    Impl(SwXTextField & rThis, SwDoc *const pDoc, SwFmtFld const*const pFmt,
+    Impl(SwDoc *const pDoc, SwFmtFld const*const pFmt,
             sal_uInt16 const nServiceId)
         : SwClient((pFmt) ? pDoc->GetUnoCallBack() : 0)
-        , m_rThis(rThis)
         , m_EventListeners(m_Mutex)
         , m_pFmtFld(pFmt)
         , m_pDoc(pDoc)
@@ -1202,7 +1187,7 @@ throw (uno::RuntimeException, std::exception)
 SwXTextField::SwXTextField(
     sal_uInt16 nServiceId,
     SwDoc* pDoc)
-    : m_pImpl(new Impl(*this, pDoc, 0, nServiceId))
+    : m_pImpl(new Impl(pDoc, 0, nServiceId))
 {
     //Set visible as default!
     if ( SW_SERVICE_FIELDTYPE_SET_EXP == nServiceId
@@ -1225,7 +1210,7 @@ SwXTextField::SwXTextField(
 SwXTextField::SwXTextField(
     const SwFmtFld& rFmt,
     SwDoc & rDoc)
-    : m_pImpl(new Impl(*this, &rDoc, &rFmt, USHRT_MAX))
+    : m_pImpl(new Impl(&rDoc, &rFmt, USHRT_MAX))
 {
 }
 
@@ -1233,6 +1218,34 @@ SwXTextField::~SwXTextField()
 {
 }
 
+uno::Reference<text::XTextField>
+SwXTextField::CreateXTextField(SwDoc *const pDoc, SwFmtFld const* pFmt,
+        sal_uInt16 const nServiceId)
+{
+    assert(!pFmt || pDoc);
+    assert(pFmt || nServiceId != 0xFFFF);
+    // re-use existing SwXTextField
+    uno::Reference<text::XTextField> xField;
+    if (pFmt)
+    {
+        xField = pFmt->GetXTextField();
+    }
+    if (!xField.is())
+    {
+        SwXTextField *const pField( (pFmt)
+                ? new SwXTextField(*pFmt, *pDoc)
+                : new SwXTextField(nServiceId, pDoc));
+        xField.set(pField);
+        if (pFmt)
+        {
+            const_cast<SwFmtFld *>(pFmt)->SetXTextField(xField);
+        }
+        // need a permanent Reference to initialize m_wThis
+        pField->m_pImpl->m_wThis = xField;
+    }
+    return xField;
+}
+
 sal_uInt16 SwXTextField::GetServiceId() const
 {
     return m_pImpl->m_nServiceId;
@@ -1317,6 +1330,7 @@ throw (lang::IllegalArgumentException, uno::RuntimeException, std::exception)
 
     SwDoc* pDoc = pRange ? (SwDoc*)pRange->GetDoc() : pCursor ? (SwDoc*)pCursor->GetDoc() : 0;
     // if a FieldMaster was attached, then the document is already fixed!
+    // NOTE: sw.SwXAutoTextEntry unoapi test depends on m_pDoc = 0 being valid
     if (!pDoc || (m_pImpl->m_pDoc && m_pImpl->m_pDoc != pDoc))
         throw lang::IllegalArgumentException();
 
@@ -2585,7 +2599,12 @@ void SwXTextField::Impl::Invalidate()
         GetRegisteredInNonConst()->Remove(this);
         m_pFmtFld = 0;
         m_pDoc = 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);
     }
 }
@@ -2990,7 +3009,7 @@ SwXFieldEnumeration::SwXFieldEnumeration(SwDoc & rDoc)
                          !pTxtFld->GetpTxtNode()->GetNodes().IsDocNodes();
             if (!bSkip)
                 pItems[ nFillPos++ ] = SwXTextField::CreateXTextField(
-                        *m_pImpl->m_pDoc, *pCurFldFmt);
+                        m_pImpl->m_pDoc, pCurFldFmt);
             pCurFldFmt = aIter.Next();
 
             // enlarge sequence if necessary
diff --git a/sw/source/core/unocore/unoportenum.cxx b/sw/source/core/unocore/unoportenum.cxx
index 7e7651cf9e2c..d79d3e3e7307 100644
--- a/sw/source/core/unocore/unoportenum.cxx
+++ b/sw/source/core/unocore/unoportenum.cxx
@@ -273,8 +273,9 @@ namespace
                     rAnnotationStartArr.insert(
                         SwAnnotationStartPortion_ImplSharedPtr(
                             new SwAnnotationStartPortion_Impl(
-                            SwXTextField::CreateXTextField( rDoc, *pAnnotationFmtFld ),
-                            rStartPos ) ) );
+                                SwXTextField::CreateXTextField(&rDoc,
+                                    pAnnotationFmtFld),
+                                rStartPos)));
                 }
             }
         }
@@ -810,7 +811,8 @@ lcl_ExportHints(
                             new SwXTextPortion(
                                 pUnoCrsr, xParent, PORTION_FIELD);
                         Reference<XTextField> const xField =
-                            SwXTextField::CreateXTextField(*pDoc, pAttr->GetFmtFld());
+                            SwXTextField::CreateXTextField(pDoc,
+                                    &pAttr->GetFmtFld());
                         pPortion->SetTextField(xField);
                     }
                     break;
@@ -835,7 +837,8 @@ lcl_ExportHints(
                         {
                             SwXTextPortion* pPortion = new SwXTextPortion( pUnoCrsr, xParent, PORTION_ANNOTATION );
                             Reference<XTextField> xField =
-                                SwXTextField::CreateXTextField(*pDoc, pAttr->GetFmtFld());
+                                SwXTextField::CreateXTextField(pDoc,
+                                        &pAttr->GetFmtFld());
                             pPortion->SetTextField(xField);
                             xRef = pPortion;
                         }
@@ -857,7 +860,8 @@ lcl_ExportHints(
                             new SwXTextPortion( pUnoCrsr, xParent, PORTION_FIELD);
                         xRef = pPortion;
                         Reference<XTextField> xField =
-                            SwXTextField::CreateXTextField(*pDoc, pAttr->GetFmtFld());
+                            SwXTextField::CreateXTextField(pDoc,
+                                    &pAttr->GetFmtFld());
                         pPortion->SetTextField(xField);
                     }
                     break;