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

10 files changed, 140 insertions, 113 deletions

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<char>(
+				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<XMLCh>("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<XMLCh>("result"))
 				);
 
 				switch ( boost::filesystem::status(p).type() ) {
 					case boost::filesystem::regular_file: {
 						itemNode->setAttribute(
-							*XercesStringGuard("type"),
-							*XercesStringGuard("file")
+							*XercesStringGuard<XMLCh>("type"),
+							*XercesStringGuard<XMLCh>("file")
 						);
 
 						break;
 					};
 					case boost::filesystem::directory_file: {
 						itemNode->setAttribute(
-							*XercesStringGuard("type"),
-							*XercesStringGuard("directory")
+							*XercesStringGuard<XMLCh>("type"),
+							*XercesStringGuard<XMLCh>("directory")
 						);
 
 						break;
 					};
 					default: {
 						itemNode->setAttribute(
-							*XercesStringGuard("type"),
-							*XercesStringGuard("misc")
+							*XercesStringGuard<XMLCh>("type"),
+							*XercesStringGuard<XMLCh>("misc")
 						);
 
 						break;
@@ -79,7 +87,7 @@ xalan::XObjectPtr FunctionReadDirectory::execute(
 
 				xercesc::DOMText* const textNode(
 					domDocument->createTextNode(
-						*XercesStringGuard(p.filename().string())
+						*XercesStringGuard<XMLCh>(p.filename().string())
 					)
 				);
 
@@ -88,11 +96,16 @@ xalan::XObjectPtr FunctionReadDirectory::execute(
 			});
 		} else {
 			xercesc::DOMElement* const resultNode(
-				domDocument->createElement(*XercesStringGuard("error"))
+				domDocument->createElement(*XercesStringGuard<XMLCh>("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<char>(
+				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<XMLCh>("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<XMLCh>("result"))
 			);
 
 			resultNode->setAttribute(
-				*XercesStringGuard("name"),
-				*XercesStringGuard(filePath.filename().string())
+				*XercesStringGuard<XMLCh>("name"),
+				*XercesStringGuard<XMLCh>(filePath.filename().string())
 			);
 
 			xercesc::DOMText* const resultTextNode(
 				domDocument->createTextNode(
-					*XercesStringGuard(readFile(filePath))
+					*XercesStringGuard<XMLCh>(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<XMLCh>("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<char>(
+				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<XMLCh>("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<XMLCh>("result"))
 			);
 
 			resultNode->setAttribute(
-				*XercesStringGuard("name"),
-				*XercesStringGuard(filePath.filename().string())
+				*XercesStringGuard<XMLCh>("name"),
+				*XercesStringGuard<XMLCh>(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<XMLCh>("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<item>(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<std::mutex> guard(this->write_mutex_);
+
+	auto result = this->map_.emplace(
+		std::make_pair(key, std::unique_ptr<item>(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 <unordered_map>
+#include <xercesc/dom/DOMDocument.hpp>
+
 #include <string>
+#include <mutex>
+#include <unordered_map>
 #include <memory>
 
 namespace InputXSLT {
@@ -10,12 +13,15 @@ namespace InputXSLT {
 class DomDocumentCache {
 	public:
 		class item;
+		typedef std::pair<bool, item*> 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<std::string, std::unique_ptr<item>> 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 <xercesc/dom/DOMImplementation.hpp>
-#include <xercesc/util/XMLString.hpp>
-
-#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<bool>(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 <xercesc/dom/DOMDocument.hpp>
 
-#include <string>
-#include <memory>
-
 #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<xalan::XercesDOMWrapperParsedSource> 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<XMLCh**>(&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 <typename Type>
 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<decltype(
+					xercesc::XMLString::transcode(new Type[1])
+				)>::type
+			>& src
+		):
+			string_(xercesc::XMLString::transcode(src.data())) { }
+
+		~XercesStringGuard() {
+			xercesc::XMLString::release(
+				const_cast<Type**>(&this->string_)
+			);
+		}
+
+		inline const Type* operator*() const {
+			return this->string_;
+		}
 
 	private:
-		XMLCh* const string_;
+		const Type* const string_;
 
 };