fix a set of race conditions in the writer uno wrappers

- whenever SwClients are added or removed, the SolarMutex should be
locked
- locking the mutex there would be a performance killer
- thus only DBG_TESTSOLARMUTEX() and fixing the fallout on DBG_UTL
builds

Change-Id: I3b10b9a01c40fbe68d15ce6e9c5c74db34eb1eb6
Reviewed-on: https://gerrit.libreoffice.org/19856

8 files changed, 35 insertions, 4 deletions

diff --git a/sw/source/core/access/accpara.cxx b/sw/source/core/access/accpara.cxx
index dce86d889887..5ab1d0d6de78 100644
--- a/sw/source/core/access/accpara.cxx
+++ b/sw/source/core/access/accpara.cxx
@@ -564,6 +564,8 @@ SwAccessibleParagraph::~SwAccessibleParagraph()
     delete pPortionData;
     delete pHyperTextData;
     delete mpParaChangeTrackInfo; // #i108125#
+    if(GetRegisteredIn())
+        GetRegisteredIn()->Remove(this);
 }
 
 bool SwAccessibleParagraph::HasCursor()
diff --git a/sw/source/core/attr/calbck.cxx b/sw/source/core/attr/calbck.cxx
index fafb9a6a1b49..0ba6a74558e2 100644
--- a/sw/source/core/attr/calbck.cxx
+++ b/sw/source/core/attr/calbck.cxx
@@ -22,6 +22,7 @@
 #include <hints.hxx>
 #include <swcache.hxx>
 #include <swfntcch.hxx>
+#include <tools/debug.hxx>
 
 sw::LegacyModifyHint::~LegacyModifyHint() {}
 
@@ -29,6 +30,8 @@ TYPEINIT0( SwClient );
 
 SwClient::~SwClient()
 {
+    if(GetRegisteredIn())
+        DBG_TESTSOLARMUTEX();
     OSL_ENSURE( !pRegisteredIn || pRegisteredIn->HasWriterListeners(), "SwModify still known, but Client already disconnected!" );
     if( pRegisteredIn && pRegisteredIn->HasWriterListeners() )
         pRegisteredIn->Remove( this );
@@ -36,6 +39,7 @@ SwClient::~SwClient()
 
 void SwClient::CheckRegistration( const SfxPoolItem* pOld, const SfxPoolItem* )
 {
+    DBG_TESTSOLARMUTEX();
     // this method only handles notification about dying SwModify objects
     if( (!pOld || pOld->Which() != RES_OBJECTDYING) )
         return;
@@ -73,6 +77,7 @@ void SwClient::Modify(SfxPoolItem const*const pOldValue, SfxPoolItem const*const
 
 SwModify::~SwModify()
 {
+    DBG_TESTSOLARMUTEX();
     OSL_ENSURE( !IsModifyLocked(), "Modify destroyed but locked." );
 
     if ( IsInCache() )
@@ -109,6 +114,7 @@ SwModify::~SwModify()
 
 void SwModify::NotifyClients( const SfxPoolItem* pOldValue, const SfxPoolItem* pNewValue )
 {
+    DBG_TESTSOLARMUTEX();
     if ( IsInCache() || IsInSwFntCache() )
     {
         const sal_uInt16 nWhich = pOldValue ? pOldValue->Which() :
@@ -158,6 +164,7 @@ bool SwModify::GetInfo( SfxPoolItem& rInfo ) const
 
 void SwModify::Add( SwClient* pDepend )
 {
+    DBG_TESTSOLARMUTEX();
     OSL_ENSURE( !m_bLockClientList, "Client inserted while in Modify" );
 
     if(pDepend->pRegisteredIn != this )
@@ -202,6 +209,7 @@ SwClient* SwModify::Remove( SwClient* pDepend )
     if(m_bInDocDTOR)
         return nullptr;
 
+    DBG_TESTSOLARMUTEX();
     assert(pDepend->pRegisteredIn == this);
 
     // SwClient is my listener
diff --git a/sw/source/core/unocore/unodraw.cxx b/sw/source/core/unocore/unodraw.cxx
index 1062514a90de..fb99025e305d 100644
--- a/sw/source/core/unocore/unodraw.cxx
+++ b/sw/source/core/unocore/unodraw.cxx
@@ -1006,12 +1006,15 @@ void SwXShape::AddExistingShapeToFormat( SdrObject& _rObj )
 
 SwXShape::~SwXShape()
 {
+    SolarMutexGuard aGuard;
     if (xShapeAgg.is())
     {
         uno::Reference< uno::XInterface >  xRef;
         xShapeAgg->setDelegator(xRef);
     }
     delete pImpl;
+    if(GetRegisteredIn())
+        GetRegisteredIn()->Remove(this);
 }
 
 uno::Any SwXShape::queryInterface( const uno::Type& aType ) throw( uno::RuntimeException, std::exception )
diff --git a/sw/source/core/unocore/unoport.cxx b/sw/source/core/unocore/unoport.cxx
index 480e76a1c627..f3f803977274 100644
--- a/sw/source/core/unocore/unoport.cxx
+++ b/sw/source/core/unocore/unoport.cxx
@@ -137,7 +137,10 @@ SwXTextPortion::SwXTextPortion(
 }
 
 SwXTextPortion::~SwXTextPortion()
-{ }
+{
+    SolarMutexGuard aGuard;
+    m_pUnoCursor.reset(nullptr);
+}
 
 uno::Reference< text::XText >  SwXTextPortion::getText()
 throw( uno::RuntimeException, std::exception )
diff --git a/sw/source/core/unocore/unoportenum.cxx b/sw/source/core/unocore/unoportenum.cxx
index c9c4e6e3973c..7af952afd233 100644
--- a/sw/source/core/unocore/unoportenum.cxx
+++ b/sw/source/core/unocore/unoportenum.cxx
@@ -384,7 +384,10 @@ SwXTextPortionEnumeration::SwXTextPortionEnumeration(
 }
 
 SwXTextPortionEnumeration::~SwXTextPortionEnumeration()
-{ }
+{
+    SolarMutexGuard aGuard;
+    m_pUnoCrsr.reset(nullptr);
+}
 
 sal_Bool SwXTextPortionEnumeration::hasMoreElements()
 throw( uno::RuntimeException, std::exception )
diff --git a/sw/source/core/unocore/unosett.cxx b/sw/source/core/unocore/unosett.cxx
index 7e4295e57889..6ec34a4fc57a 100644
--- a/sw/source/core/unocore/unosett.cxx
+++ b/sw/source/core/unocore/unosett.cxx
@@ -2099,6 +2099,7 @@ void SwXNumberingRules::setPropertyValue( const OUString& rPropertyName, const A
     throw(UnknownPropertyException, PropertyVetoException,
         IllegalArgumentException, WrappedTargetException, RuntimeException, std::exception)
 {
+    SolarMutexGuard aGuard;
     SwNumRule* pDocRule = 0;
     SwNumRule* pCreatedRule = 0;
     if(!pNumRule)
diff --git a/sw/source/core/unocore/unostyle.cxx b/sw/source/core/unocore/unostyle.cxx
index dfb10e05de3a..f615d0dad9ba 100644
--- a/sw/source/core/unocore/unostyle.cxx
+++ b/sw/source/core/unocore/unostyle.cxx
@@ -1304,6 +1304,8 @@ SwXStyle::~SwXStyle()
     if(m_pBasePool)
         EndListening(*m_pBasePool);
     delete m_pPropertiesImpl;
+    if(GetRegisteredIn())
+        GetRegisteredIn()->Remove( this );
 }
 
 void SwXStyle::Modify( const SfxPoolItem* pOld, const SfxPoolItem *pNew)
diff --git a/sw/source/core/unocore/unotbl.cxx b/sw/source/core/unocore/unotbl.cxx
index 7eb2ed73d7d2..ea8983ce0bbc 100644
--- a/sw/source/core/unocore/unotbl.cxx
+++ b/sw/source/core/unocore/unotbl.cxx
@@ -1242,7 +1242,11 @@ SwXTextTableRow::SwXTextTableRow(SwFrameFormat* pFormat, SwTableLine* pLn) :
 { }
 
 SwXTextTableRow::~SwXTextTableRow()
-{ }
+{
+    SolarMutexGuard aGuard;
+    if(GetRegisteredIn())
+        GetRegisteredIn()->Remove(this);
+}
 
 uno::Reference< beans::XPropertySetInfo > SwXTextTableRow::getPropertySetInfo() throw( uno::RuntimeException, std::exception )
 {
@@ -1960,7 +1964,12 @@ SwXTextTable::SwXTextTable(SwFrameFormat& rFrameFormat)
 { }
 
 SwXTextTable::~SwXTextTable()
-    { delete pTableProps; }
+{
+    SolarMutexGuard aGuard;
+    delete pTableProps;
+    if(GetRegisteredIn())
+        GetRegisteredIn()->Remove(this);
+}
 
 uno::Reference<text::XTextTable> SwXTextTable::CreateXTextTable(SwFrameFormat* const pFrameFormat)
 {