From 59003de73eff0da22d01f2fd3cddc78bf3a3a3f8 Mon Sep 17 00:00:00 2001 From: Michael Meeks Date: Fri, 20 Dec 2013 16:18:48 +0000 Subject: 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 --- sax/source/fastparser/fastparser.cxx | 145 +++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 68 deletions(-) (limited to 'sax') 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(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(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; } -- cgit