[Libreoffice-commits] core.git: Branch 'libreoffice-5-4' - sax/source

[Michael Stahl]

 sax/source/fastparser/fastparser.cxx |   33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

New commits:
commit 3b81d36d58172695021e3109d2490b937c3908b3
Author: Michael Stahl <mstahl at redhat.com>
Date:   Fri Jan 5 15:36:59 2018 +0100

    ofz#4392 sax: guard access to Entity::maSavedException with mutex
    
    The problem here is presumably that the parser thread reports a
    low-level SAX exception and the main thread reports a high-level filter
    exception at the same time, so both threads modify maSavedException
    concurrently.
    
    Reviewed-on: https://gerrit.libreoffice.org/47478
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Jenkins <ci at libreoffice.org>
    (cherry picked from commit 2a88f62ac01daa72b6287a8cedccfb78579a6067)
    
    Change-Id: Ic8ce9a4992208a24a111c990a67be163858ddaf8
    Reviewed-on: https://gerrit.libreoffice.org/47542
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/sax/source/fastparser/fastparser.cxx b/sax/source/fastparser/fastparser.cxx
index bbe7748b149a..a8d915cbd117 100644
--- a/sax/source/fastparser/fastparser.cxx
+++ b/sax/source/fastparser/fastparser.cxx
@@ -174,6 +174,7 @@ struct Entity : public ParserData
     // resource leaks), therefore any exception thrown by a UNO callback
     // must be saved somewhere until the C-XmlParser is stopped.
     css::uno::Any                           maSavedException;
+    osl::Mutex maSavedExceptionMutex;
     void saveException( const Any & e );
     void throwException( const ::rtl::Reference< FastLocatorImpl > &xDocumentLocator,
                          bool mbDuringParse );
@@ -586,12 +587,20 @@ void Entity::throwException( const ::rtl::Reference< FastLocatorImpl > &xDocumen
                              bool mbDuringParse )
 {
     // Error during parsing !
+    Any savedException;
+    {
+        osl::MutexGuard g(maSavedExceptionMutex);
+        if (maSavedException.hasValue())
+        {
+            savedException.setValue(&maSavedException, cppu::UnoType<decltype(maSavedException)>::get());
+        }
+    }
     SAXParseException aExcept(
         lclGetErrorMessage( mpParser,
                             xDocumentLocator->getSystemId(),
                             xDocumentLocator->getLineNumber() ),
         Reference< XInterface >(),
-        Any( &maSavedException, cppu::UnoType<decltype(maSavedException)>::get() ),
+        savedException,
         xDocumentLocator->getPublicId(),
         xDocumentLocator->getSystemId(),
         xDocumentLocator->getLineNumber(),
@@ -623,7 +632,15 @@ void Entity::saveException( const Any & e )
     // for XComponent; and yet expect to continue parsing.
     SAL_WARN("sax", "Unexpected exception from XML parser "
             << e.get<Exception>().Message);
-    maSavedException = e;
+    osl::MutexGuard g(maSavedExceptionMutex);
+    if (maSavedException.hasValue())
+    {
+        SAL_INFO("sax.fastparser", "discarding exception, already have one");
+    }
+    else
+    {
+        maSavedException = e;
+    }
 }
 
 } // namespace
@@ -842,6 +859,8 @@ void FastSaxParserImpl::parseStream(const InputSource& maStructSource)
         deleteUsedEvents();
 
         // callbacks used inside XML_Parse may have caught an exception
+        // No need to lock maSavedExceptionMutex here because parser
+        // thread is joined.
         if( rEntity.maSavedException.hasValue() )
             rEntity.throwException( mxDocumentLocator, true );
     }
@@ -1050,8 +1069,16 @@ void FastSaxParserImpl::parse()
         }
 
         // callbacks used inside XML_Parse may have caught an exception
-        if( !bContinue || rEntity.maSavedException.hasValue() )
+        if (!bContinue)
+        {
             rEntity.throwException( mxDocumentLocator, true );
+        }
+        osl::ClearableMutexGuard g(rEntity.maSavedExceptionMutex);
+        if (rEntity.maSavedException.hasValue())
+        {
+            g.clear();
+            rEntity.throwException( mxDocumentLocator, true );
+        }
     } while( nRead > 0 );
     rEntity.getEvent( DONE );
     if( rEntity.mbEnableThreads )