fastparser: fix load regression

Remove erroneous assert: maSavedException is indeed empty for
XML parser reported exceptions. Clean cut/paste code, and comment.

Change-Id: Ia538bcc87a7efcd079d3021e00ac4d2eb62f3e8d

1 files changed, 77 insertions, 68 deletions

diff --git a/sax/source/fastparser/fastparser.cxx b/sax/source/fastparser/fastparser.cxx
index 04cfbee960a9..3b93765c6341 100644
--- a/sax/source/fastparser/fastparser.cxx
+++ b/sax/source/fastparser/fastparser.cxx
@@ -77,12 +77,12 @@ enum CallbackType { INVALID, START_ELEMENT, END_ELEMENT, CHARACTERS, DONE, EXCEP
 
 struct Event
 {
-    OUString msChars;
+    CallbackType maType;
     sal_Int32 mnElementToken;
     OUString msNamespace;
     OUString msElementName;
     rtl::Reference< FastAttributeList > mxAttributes;
-    CallbackType maType;
+    OUString msChars;
 };
 
 struct NameWithToken
@@ -165,9 +165,13 @@ struct Entity : public ParserData
     XML_Parser                              mpParser;
     ::sax_expatwrap::XMLFile2UTFConverter   maConverter;
 
-    // Exceptions cannot be thrown through the C-XmlParser (possible resource leaks),
-    // therefore the exception must be saved somewhere.
-    ::com::sun::star::uno::Any              maSavedException;
+    // Exceptions cannot be thrown through the C-XmlParser (possible
+    // resource leaks), therefore any exception thrown by a UNO callback
+    // must be saved somewhere until the C-XmlParser is stopped.
+    ::com::sun::star::uno::Any maSavedException;
+    void saveException( const Exception &e );
+    void throwException( const ::rtl::Reference< FastLocatorImpl > &xDocumentLocator,
+                         bool mbDuringParse );
 
     ::std::stack< NameWithToken >           maNamespaceStack;
     /* Context for main thread consuming events.
@@ -272,9 +276,10 @@ private:
         {
             mpParser->parse();
         }
-        catch (const SAXParseException&)
+        catch (const Exception &e)
         {
-            mpParser->getEntity().getEvent( EXCEPTION );
+            Entity &rEntity = mpParser->getEntity();
+            rEntity.getEvent( EXCEPTION );
             mpParser->produce( EXCEPTION );
         }
     }
@@ -454,7 +459,7 @@ void Entity::startElement( Event *pEvent )
     }
     catch (const Exception& e)
     {
-        maSavedException <<= e;
+        saveException( e );
     }
 }
 
@@ -467,7 +472,7 @@ void Entity::characters( const OUString& sChars )
     }
     catch (const Exception& e)
     {
-        maSavedException <<= e;
+        saveException( e );
     }
 }
 
@@ -485,7 +490,7 @@ void Entity::endElement()
     }
     catch (const Exception& e)
     {
-        maSavedException <<= e;
+        saveException( e );
     }
     maContextStack.pop();
 }
@@ -781,7 +786,7 @@ void FastSaxParserImpl::parseStream( const InputSource& maStructSource) throw (S
     {
         popEntity();
         XML_ParserFree( entity.mpParser );
-          throw;
+        throw;
     }
     catch (const IOException&)
     {
@@ -958,7 +963,7 @@ bool FastSaxParserImpl::consume(EventList *pEventList)
 {
     Entity& rEntity = getEntity();
     for (EventList::iterator aEventIt = pEventList->begin();
-            aEventIt != pEventList->end(); ++aEventIt)
+         aEventIt != pEventList->end(); ++aEventIt)
     {
         switch ((*aEventIt).maType)
         {
@@ -974,28 +979,8 @@ bool FastSaxParserImpl::consume(EventList *pEventList)
             case DONE:
                 return false;
             case EXCEPTION:
-            {
-                assert( rEntity.maSavedException.hasValue() );
-                // Error during parsing !
-                XML_Error xmlE = XML_GetErrorCode( rEntity.mpParser );
-                OUString sSystemId = mxDocumentLocator->getSystemId();
-                sal_Int32 nLine = mxDocumentLocator->getLineNumber();
-
-                SAXParseException aExcept(
-                    lclGetErrorMessage( xmlE, sSystemId, nLine ),
-                    Reference< XInterface >(),
-                    Any( &rEntity.maSavedException, getCppuType( &rEntity.maSavedException ) ),
-                    mxDocumentLocator->getPublicId(),
-                    mxDocumentLocator->getSystemId(),
-                    mxDocumentLocator->getLineNumber(),
-                    mxDocumentLocator->getColumnNumber()
-                );
-                // error handler is set, it may throw the exception
-                if( rEntity.mxErrorHandler.is() )
-                    rEntity.mxErrorHandler->fatalError( Any( aExcept ) );
-
-                throw aExcept;
-            }
+                rEntity.throwException( mxDocumentLocator, false );
+                return false;
             default:
                 assert(false);
                 return false;
@@ -1024,6 +1009,35 @@ const Entity& FastSaxParserImpl::getEntity() const
     return maEntities.top();
 }
 
+// throw an exception, but avoid callback if
+// during a threaded produce
+void Entity::throwException( const ::rtl::Reference< FastLocatorImpl > &xDocumentLocator,
+                             bool mbDuringParse )
+{
+    // Error during parsing !
+    SAXParseException aExcept(
+        lclGetErrorMessage( XML_GetErrorCode( mpParser ),
+                            xDocumentLocator->getSystemId(),
+                            xDocumentLocator->getLineNumber() ),
+        Reference< XInterface >(),
+        Any( &maSavedException, getCppuType( &maSavedException ) ),
+        xDocumentLocator->getPublicId(),
+        xDocumentLocator->getSystemId(),
+        xDocumentLocator->getLineNumber(),
+        xDocumentLocator->getColumnNumber()
+    );
+
+    // error handler is set, it may throw the exception
+    if( !mbDuringParse || !mbEnableThreads )
+    {
+        if (mxErrorHandler.is() )
+            mxErrorHandler->fatalError( Any( aExcept ) );
+    }
+
+    // error handler has not thrown, but parsing must stop => throw ourselves
+    throw aExcept;
+}
+
 // starts parsing with actual parser !
 void FastSaxParserImpl::parse()
 {
@@ -1043,39 +1057,34 @@ void FastSaxParserImpl::parse()
 
         bool const bContinue = XML_STATUS_ERROR != XML_Parse(rEntity.mpParser,
             reinterpret_cast<const char*>(seqOut.getConstArray()), nRead, 0);
+
         // callbacks used inside XML_Parse may have caught an exception
         if( !bContinue || rEntity.maSavedException.hasValue() )
-        {
-            // Error during parsing !
-            XML_Error xmlE = XML_GetErrorCode( rEntity.mpParser );
-            OUString sSystemId = mxDocumentLocator->getSystemId();
-            sal_Int32 nLine = mxDocumentLocator->getLineNumber();
-
-            SAXParseException aExcept(
-                lclGetErrorMessage( xmlE, sSystemId, nLine ),
-                Reference< XInterface >(),
-                Any( &rEntity.maSavedException, getCppuType( &rEntity.maSavedException ) ),
-                mxDocumentLocator->getPublicId(),
-                mxDocumentLocator->getSystemId(),
-                mxDocumentLocator->getLineNumber(),
-                mxDocumentLocator->getColumnNumber()
-            );
-
-            // error handler is set, it may throw the exception
-            if( rEntity.mxErrorHandler.is() )
-                rEntity.mxErrorHandler->fatalError( Any( aExcept ) );
-
-            // error handler has not thrown, but parsing cannot go on, the
-            // exception MUST be thrown
-            throw aExcept;
-        }
+            rEntity.throwException( mxDocumentLocator, true );
     }
     while( nRead > 0 );
     rEntity.getEvent( DONE );
-    if (rEntity.mbEnableThreads)
+    if( rEntity.mbEnableThreads )
         produce( DONE );
 }
 
+// In the single threaded case we emit events via our C
+// callbacks, so any exception caught must be queued up until
+// we can safely re-throw it from our C++ parent of parse()
+//
+// If multi-threaded, we need to push an EXCEPTION event, at
+// which point we transfer ownership of maSavedException to
+// the consuming thread.
+void Entity::saveException( const Exception &e )
+{
+    // only store the first exception
+    if( !maSavedException.hasValue() )
+    {
+        maSavedException <<= e;
+        XML_StopParser( mpParser, /* resumable? */ XML_FALSE );
+    }
+}
+
 //------------------------------------------
 //
 // The C-Callbacks
@@ -1207,7 +1216,7 @@ void FastSaxParserImpl::callbackStartElement( const XML_Char* pwName, const XML_
     }
     catch (const Exception& e)
     {
-        rEntity.maSavedException <<= e;
+        rEntity.saveException( e );
     }
 }
 
@@ -1253,13 +1262,13 @@ void FastSaxParserImpl::callbackEntityDecl(
     if (value) { // value != 0 means internal entity
         SAL_INFO("sax", "FastSaxParser: internal entity declaration, stopping");
         XML_StopParser(getEntity().mpParser, XML_FALSE);
-        getEntity().maSavedException <<= SAXParseException(
+        getEntity().saveException( SAXParseException(
             "FastSaxParser: internal entity declaration, stopping",
             static_cast<OWeakObject*>(mpFront), Any(),
             mxDocumentLocator->getPublicId(),
             mxDocumentLocator->getSystemId(),
             mxDocumentLocator->getLineNumber(),
-            mxDocumentLocator->getColumnNumber() );
+            mxDocumentLocator->getColumnNumber() ) );
     } else {
         SAL_INFO("sax", "FastSaxParser: ignoring external entity declaration");
     }
@@ -1284,17 +1293,17 @@ int FastSaxParserImpl::callbackExternalEntityRef(
     }
     catch (const SAXParseException & e)
     {
-        rCurrEntity.maSavedException <<= e;
+        rCurrEntity.saveException( e );
         bOK = false;
     }
     catch (const SAXException& e)
     {
-        rCurrEntity.maSavedException <<= SAXParseException(
+        rCurrEntity.saveException( SAXParseException(
             e.Message, e.Context, e.WrappedException,
             mxDocumentLocator->getPublicId(),
             mxDocumentLocator->getSystemId(),
             mxDocumentLocator->getLineNumber(),
-            mxDocumentLocator->getColumnNumber() );
+            mxDocumentLocator->getColumnNumber() ) );
         bOK = false;
     }
 
@@ -1314,21 +1323,21 @@ int FastSaxParserImpl::callbackExternalEntityRef(
         }
         catch (const SAXParseException& e)
         {
-            rCurrEntity.maSavedException <<= e;
+            rCurrEntity.saveException( e );
             bOK = false;
         }
         catch (const IOException& e)
         {
             SAXException aEx;
             aEx.WrappedException <<= e;
-            rCurrEntity.maSavedException <<= aEx;
+            rCurrEntity.saveException( aEx );
             bOK = false;
         }
         catch (const RuntimeException& e)
         {
             SAXException aEx;
             aEx.WrappedException <<= e;
-            rCurrEntity.maSavedException <<= aEx;
+            rCurrEntity.saveException( aEx );
             bOK = false;
         }