From 261170cc5b6661106877dc2611dab281f4d91348 Mon Sep 17 00:00:00 2001 From: Adrian Kummerländer Date: Sat, 3 May 2014 22:04:16 +0200 Subject: Revamped DomDocumentCache with regard to thread safety * DomDocumentCache::item class now is _finalized_ by default and doesn't perform document instantiation, just lifetime management ** xercesc::DOMDocument is instantiated inside the external function implementations and committed to the document cache for conversion to a xalan document * added mutex with scoped lock to prevent concurrent write access to the std::unordered_map contained withing DomDocumentCache * functionality of the _get_ member method was split into _get_ and _create_ ** added typedef for std::pair specialization type "optional_item" that functions as the return value of _create_ and _get_ * "locator->getSystemId()" was leaking memory as xerces doesn't manage the lifetime of the returned heap-allocated char array ** analog to XMLCh* strings ** transformed XercesStringGuard into template class to be instantiated on either XMLCh or char --- CMakeLists.txt | 1 - src/function/read_directory.cc | 43 +++++++++++++++++++++++----------- src/function/read_file.cc | 35 +++++++++++++++++++-------- src/function/read_xml_file.cc | 33 +++++++++++++++++++------- src/support/dom/document_cache.cc | 32 ++++++++++++++++--------- src/support/dom/document_cache.h | 10 ++++++-- src/support/dom/document_cache_item.cc | 43 +++++++--------------------------- src/support/dom/document_cache_item.h | 10 ++------ src/support/xerces_string_guard.cc | 18 -------------- src/support/xerces_string_guard.h | 28 ++++++++++++++++++---- 10 files changed, 140 insertions(+), 113 deletions(-) delete mode 100644 src/support/xerces_string_guard.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index fd14740..565b3cb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -21,7 +21,6 @@ add_executable( src/support/filesystem_context.cc src/support/dom/document_cache.cc src/support/dom/document_cache_item.cc - src/support/xerces_string_guard.cc ) target_link_libraries( diff --git a/src/function/read_directory.cc b/src/function/read_directory.cc index b13be2b..f8f99d6 100644 --- a/src/function/read_directory.cc +++ b/src/function/read_directory.cc @@ -21,7 +21,11 @@ xalan::XObjectPtr FunctionReadDirectory::execute( ) const { const FilesystemContext fs_context( boost::filesystem::path( - xercesc::XMLString::transcode(locator->getSystemId() + 7) + *XercesStringGuard( + xercesc::XMLString::transcode( + locator->getSystemId() + 7 + ) + ) ).parent_path().string() ); @@ -29,13 +33,17 @@ xalan::XObjectPtr FunctionReadDirectory::execute( fs_context.resolve(argument->str()) ); - DomDocumentCache::item* const cachedDocument( + DomDocumentCache::optional_item optionalCachedDocument( this->document_cache_->get(directoryPath.string()) ); - if ( !cachedDocument->isFinalized() ) { + if ( !optionalCachedDocument.first ) { xercesc::DOMDocument* const domDocument( - cachedDocument->getXercesDocument() + xercesc::DOMImplementation::getImplementation()->createDocument( + nullptr, + *XercesStringGuard("content"), + nullptr + ) ); xercesc::DOMNode* const rootNode( @@ -47,30 +55,30 @@ xalan::XObjectPtr FunctionReadDirectory::execute( argument->str(), [&domDocument, &rootNode](const boost::filesystem::path& p) { xercesc::DOMElement* const itemNode( - domDocument->createElement(*XercesStringGuard("result")) + domDocument->createElement(*XercesStringGuard("result")) ); switch ( boost::filesystem::status(p).type() ) { case boost::filesystem::regular_file: { itemNode->setAttribute( - *XercesStringGuard("type"), - *XercesStringGuard("file") + *XercesStringGuard("type"), + *XercesStringGuard("file") ); break; }; case boost::filesystem::directory_file: { itemNode->setAttribute( - *XercesStringGuard("type"), - *XercesStringGuard("directory") + *XercesStringGuard("type"), + *XercesStringGuard("directory") ); break; }; default: { itemNode->setAttribute( - *XercesStringGuard("type"), - *XercesStringGuard("misc") + *XercesStringGuard("type"), + *XercesStringGuard("misc") ); break; @@ -79,7 +87,7 @@ xalan::XObjectPtr FunctionReadDirectory::execute( xercesc::DOMText* const textNode( domDocument->createTextNode( - *XercesStringGuard(p.filename().string()) + *XercesStringGuard(p.filename().string()) ) ); @@ -88,11 +96,16 @@ xalan::XObjectPtr FunctionReadDirectory::execute( }); } else { xercesc::DOMElement* const resultNode( - domDocument->createElement(*XercesStringGuard("error")) + domDocument->createElement(*XercesStringGuard("error")) ); rootNode->appendChild(resultNode); } + + optionalCachedDocument = this->document_cache_->create( + directoryPath.string(), + domDocument + ); } xalan::XPathExecutionContext::BorrowReturnMutableNodeRefList nodeList( @@ -100,7 +113,9 @@ xalan::XObjectPtr FunctionReadDirectory::execute( ); nodeList->addNodes( - *cachedDocument->getXalanDocument()->getDocumentElement()->getChildNodes() + *optionalCachedDocument.second->getXalanDocument() + ->getDocumentElement() + ->getChildNodes() ); return executionContext.getXObjectFactory().createNodeSet(nodeList); diff --git a/src/function/read_file.cc b/src/function/read_file.cc index 7634d54..633559e 100644 --- a/src/function/read_file.cc +++ b/src/function/read_file.cc @@ -36,7 +36,11 @@ xalan::XObjectPtr FunctionReadFile::execute( ) const { const FilesystemContext fs_context( boost::filesystem::path( - xercesc::XMLString::transcode(locator->getSystemId() + 7) + *XercesStringGuard( + xercesc::XMLString::transcode( + locator->getSystemId() + 7 + ) + ) ).parent_path().string() ); @@ -44,13 +48,17 @@ xalan::XObjectPtr FunctionReadFile::execute( fs_context.resolve(argument->str()) ); - DomDocumentCache::item* const cachedDocument( + DomDocumentCache::optional_item optionalCachedDocument( this->document_cache_->get(filePath.string()) ); - if ( !cachedDocument->isFinalized() ) { + if ( !optionalCachedDocument.first ) { xercesc::DOMDocument* const domDocument( - cachedDocument->getXercesDocument() + xercesc::DOMImplementation::getImplementation()->createDocument( + nullptr, + *XercesStringGuard("content"), + nullptr + ) ); xercesc::DOMNode* const rootNode( @@ -59,17 +67,17 @@ xalan::XObjectPtr FunctionReadFile::execute( if ( boost::filesystem::is_regular_file(filePath) ) { xercesc::DOMElement* const resultNode( - domDocument->createElement(*XercesStringGuard("result")) + domDocument->createElement(*XercesStringGuard("result")) ); resultNode->setAttribute( - *XercesStringGuard("name"), - *XercesStringGuard(filePath.filename().string()) + *XercesStringGuard("name"), + *XercesStringGuard(filePath.filename().string()) ); xercesc::DOMText* const resultTextNode( domDocument->createTextNode( - *XercesStringGuard(readFile(filePath)) + *XercesStringGuard(readFile(filePath)) ) ); @@ -77,11 +85,16 @@ xalan::XObjectPtr FunctionReadFile::execute( rootNode->appendChild(resultNode); } else { xercesc::DOMElement* const resultNode( - domDocument->createElement(*XercesStringGuard("error")) + domDocument->createElement(*XercesStringGuard("error")) ); rootNode->appendChild(resultNode); } + + optionalCachedDocument = this->document_cache_->create( + filePath.string(), + domDocument + ); } xalan::XPathExecutionContext::BorrowReturnMutableNodeRefList nodeList( @@ -89,7 +102,9 @@ xalan::XObjectPtr FunctionReadFile::execute( ); nodeList->addNodes( - *cachedDocument->getXalanDocument()->getDocumentElement()->getChildNodes() + *optionalCachedDocument.second->getXalanDocument() + ->getDocumentElement() + ->getChildNodes() ); return executionContext.getXObjectFactory().createNodeSet(nodeList); diff --git a/src/function/read_xml_file.cc b/src/function/read_xml_file.cc index 608e71a..dc98fc7 100644 --- a/src/function/read_xml_file.cc +++ b/src/function/read_xml_file.cc @@ -42,7 +42,11 @@ xalan::XObjectPtr FunctionReadXmlFile::execute( ) const { const FilesystemContext fs_context( boost::filesystem::path( - xercesc::XMLString::transcode(locator->getSystemId() + 7) + *XercesStringGuard( + xercesc::XMLString::transcode( + locator->getSystemId() + 7 + ) + ) ).parent_path().string() ); @@ -50,13 +54,17 @@ xalan::XObjectPtr FunctionReadXmlFile::execute( fs_context.resolve(argument->str()) ); - DomDocumentCache::item* const cachedDocument( + DomDocumentCache::optional_item optionalCachedDocument( this->document_cache_->get(filePath.string()) ); - if ( !cachedDocument->isFinalized() ) { + if ( !optionalCachedDocument.first ) { xercesc::DOMDocument* const domDocument( - cachedDocument->getXercesDocument() + xercesc::DOMImplementation::getImplementation()->createDocument( + nullptr, + *XercesStringGuard("content"), + nullptr + ) ); xercesc::DOMNode* const rootNode( @@ -65,12 +73,12 @@ xalan::XObjectPtr FunctionReadXmlFile::execute( if ( boost::filesystem::is_regular_file(filePath) ) { xercesc::DOMElement* const resultNode( - domDocument->createElement(*XercesStringGuard("result")) + domDocument->createElement(*XercesStringGuard("result")) ); resultNode->setAttribute( - *XercesStringGuard("name"), - *XercesStringGuard(filePath.filename().string()) + *XercesStringGuard("name"), + *XercesStringGuard(filePath.filename().string()) ); xercesc::DOMNode* const resultTreeNode( @@ -81,11 +89,16 @@ xalan::XObjectPtr FunctionReadXmlFile::execute( rootNode->appendChild(resultNode); } else { xercesc::DOMElement* const resultNode( - domDocument->createElement(*XercesStringGuard("error")) + domDocument->createElement(*XercesStringGuard("error")) ); rootNode->appendChild(resultNode); } + + optionalCachedDocument = this->document_cache_->create( + filePath.string(), + domDocument + ); } xalan::XPathExecutionContext::BorrowReturnMutableNodeRefList nodeList( @@ -93,7 +106,9 @@ xalan::XObjectPtr FunctionReadXmlFile::execute( ); nodeList->addNodes( - *cachedDocument->getXalanDocument()->getDocumentElement()->getChildNodes() + *optionalCachedDocument.second->getXalanDocument() + ->getDocumentElement() + ->getChildNodes() ); return executionContext.getXObjectFactory().createNodeSet(nodeList); diff --git a/src/support/dom/document_cache.cc b/src/support/dom/document_cache.cc index 1573af8..c4c57d3 100644 --- a/src/support/dom/document_cache.cc +++ b/src/support/dom/document_cache.cc @@ -5,23 +5,33 @@ namespace InputXSLT { DomDocumentCache::DomDocumentCache(): + write_mutex_(), map_() { } -DomDocumentCache::item* DomDocumentCache::get(const std::string& key) { +DomDocumentCache::optional_item DomDocumentCache::get(const std::string& key) { auto itemIter = this->map_.find(key); if ( itemIter == this->map_.end() ) { - auto result = this->map_.emplace( - std::make_pair(key, std::unique_ptr(new item("content"))) - ); - - if ( result.second ) { - return (*(result.first)).second.get(); - } else { - throw std::out_of_range("failed to instantiate DomDocumentCache"); - } + return optional_item(false, nullptr); } else { - return (*itemIter).second.get(); + return optional_item(true, (*itemIter).second.get()); + } +} + +DomDocumentCache::optional_item DomDocumentCache::create( + const std::string& key, + xercesc::DOMDocument* document +) { + std::lock_guard guard(this->write_mutex_); + + auto result = this->map_.emplace( + std::make_pair(key, std::unique_ptr(new item(document))) + ); + + if ( result.second ) { + return optional_item(true, (*(result.first)).second.get()); + } else { + return optional_item(false, nullptr); } } diff --git a/src/support/dom/document_cache.h b/src/support/dom/document_cache.h index 287a316..af99102 100644 --- a/src/support/dom/document_cache.h +++ b/src/support/dom/document_cache.h @@ -1,8 +1,11 @@ #ifndef INPUTXSLT_SRC_SUPPORT_DOM_DOCUMENT_CACHE_H_ #define INPUTXSLT_SRC_SUPPORT_DOM_DOCUMENT_CACHE_H_ -#include +#include + #include +#include +#include #include namespace InputXSLT { @@ -10,12 +13,15 @@ namespace InputXSLT { class DomDocumentCache { public: class item; + typedef std::pair optional_item; DomDocumentCache(); - item* get(const std::string&); + optional_item get(const std::string&); + optional_item create(const std::string&, xercesc::DOMDocument*); private: + std::mutex write_mutex_; std::unordered_map> map_; }; diff --git a/src/support/dom/document_cache_item.cc b/src/support/dom/document_cache_item.cc index 8cc1c24..9798bfa 100644 --- a/src/support/dom/document_cache_item.cc +++ b/src/support/dom/document_cache_item.cc @@ -1,50 +1,23 @@ #include "support/dom/document_cache_item.h" -#include -#include - -#include "support/xerces_string_guard.h" - namespace InputXSLT { -DomDocumentCache::item::item(const std::string& rootNode): +DomDocumentCache::item::item(xercesc::DOMDocument* document): parser_(), dom_support_(parser_), - document_( - xercesc::DOMImplementation::getImplementation()->createDocument( - nullptr, - *XercesStringGuard(rootNode), - nullptr - ) - ), - parsed_source_() { } + document_(document), + parsed_source_( + document_, + parser_, + dom_support_ + ) { } DomDocumentCache::item::~item() { this->document_->release(); } -bool DomDocumentCache::item::isFinalized() const { - return static_cast(this->parsed_source_); -} - -xercesc::DOMDocument* DomDocumentCache::item::getXercesDocument() const { - return this->document_; -} - xalan::XalanDocument* DomDocumentCache::item::getXalanDocument() { - if ( this->parsed_source_ ) { - return this->parsed_source_->getDocument(); - } else { - this->parsed_source_.reset( - new xalan::XercesDOMWrapperParsedSource( - document_, - parser_, - dom_support_ - ) - ); - - return this->parsed_source_->getDocument(); - } + return this->parsed_source_.getDocument(); } } diff --git a/src/support/dom/document_cache_item.h b/src/support/dom/document_cache_item.h index 1f7152b..13fddfc 100644 --- a/src/support/dom/document_cache_item.h +++ b/src/support/dom/document_cache_item.h @@ -7,9 +7,6 @@ #include -#include -#include - #include "common.h" #include "document_cache.h" @@ -19,21 +16,18 @@ class DomDocumentCache::item { public: ~item(); - bool isFinalized() const; - - xercesc::DOMDocument* getXercesDocument() const; xalan::XalanDocument* getXalanDocument(); protected: friend DomDocumentCache; - item(const std::string&); + item(xercesc::DOMDocument*); private: xalan::XercesParserLiaison parser_; xalan::XercesDOMSupport dom_support_; xercesc::DOMDocument* const document_; - std::unique_ptr parsed_source_; + xalan::XercesDOMWrapperParsedSource parsed_source_; }; diff --git a/src/support/xerces_string_guard.cc b/src/support/xerces_string_guard.cc deleted file mode 100644 index 14ad29b..0000000 --- a/src/support/xerces_string_guard.cc +++ /dev/null @@ -1,18 +0,0 @@ -#include "xerces_string_guard.h" - -namespace InputXSLT { - -XercesStringGuard::XercesStringGuard(const std::string& src): - string_(xercesc::XMLString::transcode(src.data())) { } - -XercesStringGuard::~XercesStringGuard() { - xercesc::XMLString::release( - const_cast(&this->string_) - ); -} - -XMLCh* XercesStringGuard::operator*() { - return this->string_; -} - -} diff --git a/src/support/xerces_string_guard.h b/src/support/xerces_string_guard.h index f64a53e..7b9517a 100644 --- a/src/support/xerces_string_guard.h +++ b/src/support/xerces_string_guard.h @@ -7,15 +7,33 @@ namespace InputXSLT { +template class XercesStringGuard { public: - XercesStringGuard(const std::string&); - ~XercesStringGuard(); - - XMLCh* operator*(); + XercesStringGuard(const Type* src): + string_(src) { } + + XercesStringGuard( + const std::basic_string< + typename std::remove_pointer::type + >& src + ): + string_(xercesc::XMLString::transcode(src.data())) { } + + ~XercesStringGuard() { + xercesc::XMLString::release( + const_cast(&this->string_) + ); + } + + inline const Type* operator*() const { + return this->string_; + } private: - XMLCh* const string_; + const Type* const string_; }; -- cgit v1.2.3