cid#1608473 Data race condition

which requires some care, because these objects are linked
together into a double-linked list, and they call back into
each other.

Change-Id: If5c128918a1ae901ab7789067dc47d30f7415b43
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174705
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

io/source/stm/omark.cxx

@@ -445,14 +445,15 @@ OMarkableInputStream::OMarkableInputStream()
 
 sal_Int32 OMarkableInputStream::readBytes(Sequence< sal_Int8 >& aData, sal_Int32 nBytesToRead)
 {
-    sal_Int32 nBytesRead;
+    std::unique_lock guard( m_mutex );
 
     if( !m_bValidStream ) {
         throw NotConnectedException(
             u"MarkableInputStream::readBytes NotConnectedException"_ustr,
             *this );
     }
-    std::unique_lock guard( m_mutex );
+
+    sal_Int32 nBytesRead;
     if( m_mapMarks.empty() && ! m_oBuffer->getSize() ) {
         // normal read !
         nBytesRead = m_input->readBytes( aData, nBytesToRead );
@@ -489,15 +490,15 @@ sal_Int32 OMarkableInputStream::readBytes(Sequence< sal_Int8 >& aData, sal_Int32
 
 sal_Int32 OMarkableInputStream::readSomeBytes(Sequence< sal_Int8 >& aData, sal_Int32 nMaxBytesToRead)
 {
+    std::unique_lock guard( m_mutex );
 
-    sal_Int32 nBytesRead;
     if( !m_bValidStream )    {
         throw NotConnectedException(
             u"MarkableInputStream::readSomeBytes NotConnectedException"_ustr,
             *this );
     }
 
-    std::unique_lock guard( m_mutex );
+    sal_Int32 nBytesRead;
     if( m_mapMarks.empty() && ! m_oBuffer->getSize() ) {
         // normal read !
         nBytesRead = m_input->readSomeBytes( aData, nMaxBytesToRead );
@@ -551,13 +552,14 @@ void OMarkableInputStream::skipBytes(sal_Int32 nBytesToSkip)
 
 sal_Int32 OMarkableInputStream::available()
 {
+    std::unique_lock guard( m_mutex );
+
     if( !m_bValidStream )    {
         throw NotConnectedException(
             u"MarkableInputStream::available NotConnectedException"_ustr,
             *this );
     }
 
-    std::unique_lock guard( m_mutex );
     sal_Int32 nAvail = m_input->available() + ( m_oBuffer->getSize() - m_nCurrentPos );
     return nAvail;
 }
@@ -565,19 +567,22 @@ sal_Int32 OMarkableInputStream::available()
 
 void OMarkableInputStream::closeInput()
 {
+    std::unique_lock guard( m_mutex );
+
     if( !m_bValidStream ) {
         throw NotConnectedException(
             u"MarkableInputStream::closeInput NotConnectedException"_ustr,
             *this );
     }
-    std::unique_lock guard( m_mutex );
 
     m_input->closeInput();
 
-    setInputStream( Reference< XInputStream > () );
-    setPredecessor( Reference< XConnectable > () );
-    setSuccessor( Reference< XConnectable >() );
-
+    m_input.clear();
+    if( m_pred )
+        m_pred.clear();
+    if( m_succ )
+        m_succ.clear();
+    m_bValidStream = false;
     m_oBuffer.reset();
     m_nCurrentPos = 0;
     m_nCurrentMark = 0;
@@ -649,20 +654,22 @@ sal_Int32 OMarkableInputStream::offsetToMark(sal_Int32 nMark)
 // XActiveDataSource
 void OMarkableInputStream::setInputStream(const Reference< XInputStream > & aStream)
 {
+    Reference < XConnectable > pred;
+    {
+        std::unique_lock guard( m_mutex );
+        if( m_input == aStream )
+            return;
 
-    if( m_input != aStream ) {
         m_input = aStream;
-
-        Reference < XConnectable >  pred( m_input , UNO_QUERY );
-        setPredecessor( pred );
+        m_bValidStream = m_input.is();
+        pred.set( m_input , UNO_QUERY );
     }
-
-    m_bValidStream = m_input.is();
-
+    setPredecessor( pred );
 }
 
 Reference< XInputStream > OMarkableInputStream::getInputStream()
 {
+    std::unique_lock guard( m_mutex );
     return m_input;
 }
 
@@ -670,21 +677,24 @@ Reference< XInputStream > OMarkableInputStream::getInputStream()
 // XDataSink
 void OMarkableInputStream::setSuccessor( const Reference< XConnectable > &r )
 {
-     /// if the references match, nothing needs to be done
-     if( m_succ != r ) {
-         /// store the reference for later use
-         m_succ = r;
+    {
+        std::unique_lock guard( m_mutex );
+        /// if the references match, nothing needs to be done
+        if( m_succ == r )
+            return;
 
-         if( m_succ.is() ) {
-              /// set this instance as the sink !
-              m_succ->setPredecessor( Reference< XConnectable > (
-                  static_cast< XConnectable * >(this) ) );
-         }
-     }
+        /// store the reference for later use
+        m_succ = r;
+    }
+    if( r ) {
+        /// set this instance as the sink !
+        r->setPredecessor( Reference< XConnectable > ( static_cast< XConnectable * >(this) ) );
+    }
 }
 
 Reference < XConnectable >  OMarkableInputStream::getSuccessor()
 {
+    std::unique_lock guard( m_mutex );
     return m_succ;
 }
 
@@ -692,16 +702,21 @@ Reference < XConnectable >  OMarkableInputStream::getSuccessor()
 // XDataSource
 void OMarkableInputStream::setPredecessor( const Reference < XConnectable >  &r )
 {
-    if( r != m_pred ) {
+    {
+        std::unique_lock guard( m_mutex );
+        if( r == m_pred )
+            return;
         m_pred = r;
-        if( m_pred.is() ) {
-            m_pred->setSuccessor( Reference< XConnectable > (
-                static_cast< XConnectable * >(this) ) );
-        }
+    }
+    if( r ) {
+        r->setSuccessor( Reference< XConnectable > (
+            static_cast< XConnectable * >(this) ) );
     }
 }
+
 Reference< XConnectable >  OMarkableInputStream::getPredecessor()
 {
+    std::unique_lock guard( m_mutex );
     return m_pred;
 }