#4182: Util: Make load()/save()/clear() operations on configurations thread-safe

Util/include/Poco/Util/AbstractConfiguration.h

@@ -421,6 +421,37 @@ public:
 		/// Returns true iff events are enabled.
 
 protected:
+	class ScopedLock
+		/// A helper class allowing to temporarily
+		/// lock an entire AbstractConfiguration,
+		/// for use by subclasses. A typical use 
+		/// case is loading or saving an entire
+		/// configuration in a thread-safe way.
+		///
+		/// Caution: Thoughtless use of this class 
+		/// may easily lead to deadlock situations
+		/// in connection with events if any of the
+		/// mutating methods (set...(), remove())
+		/// are called with the lock held. Therefore
+		/// this class is available to subclasses
+		/// only, not for general use.
+	{
+	public:
+		explicit ScopedLock(const AbstractConfiguration& c):
+			_c(c)
+		{
+			_c._mutex.lock();
+		}
+
+		~ScopedLock()
+		{
+			_c._mutex.unlock();
+		}
+
+	private:
+		const AbstractConfiguration& _c;
+	};
+
 	virtual bool getRaw(const std::string& key, std::string& value) const = 0;
 		/// If the property with the given key exists, stores the property's value
 		/// in value and returns true. Otherwise, returns false.
@@ -493,6 +524,7 @@ private:
 	friend class ConfigurationView;
 	friend class LocalConfigurationView;
 	friend class ConfigurationMapper;
+	friend class ScopedLock;
 };
 
 

Util/src/AbstractConfiguration.cpp

@@ -105,7 +105,6 @@ std::string AbstractConfiguration::getRawString(const std::string& key) const
 
 std::string AbstractConfiguration::getRawString(const std::string& key, const std::string& defaultValue) const
 {
-
 	Mutex::ScopedLock lock(_mutex);
 
 	std::string value;

Util/src/IniFileConfiguration.cpp

@@ -59,6 +59,8 @@ IniFileConfiguration::~IniFileConfiguration()
 
 void IniFileConfiguration::load(std::istream& istr)
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	_map.clear();
 	_sectionKey.clear();
 	while (!istr.eof())

Util/src/JSONConfiguration.cpp

@@ -67,6 +67,8 @@ void JSONConfiguration::load(const std::string& path)
 
 void JSONConfiguration::load(std::istream& istr)
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	JSON::Parser parser;
 	parser.parse(istr);
 	DynamicAny result = parser.result();
@@ -79,6 +81,8 @@ void JSONConfiguration::load(std::istream& istr)
 
 void JSONConfiguration::loadEmpty(const std::string& root)
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	_object = new JSON::Object();
 	JSON::Object::Ptr rootObject = new JSON::Object();
 	_object->set(root, rootObject);
@@ -106,7 +110,7 @@ void JSONConfiguration::getIndexes(std::string& name, std::vector<int>& indexes)
 	int firstOffset = -1;
 	int offset = 0;
 	RegularExpression regex("\\[([0-9]+)\\]");
-	while(regex.match(name, offset, matches) > 0)
+	while (regex.match(name, offset, matches) > 0)
 	{
 		if (firstOffset == -1)
 		{
@@ -131,7 +135,7 @@ JSON::Object::Ptr JSONConfiguration::findStart(const std::string& key, std::stri
 	StringTokenizer tokenizer(key, ".");
 	lastPart = tokenizer[tokenizer.count() - 1];
 
-	for(int i = 0; i < tokenizer.count() - 1; ++i)
+	for (int i = 0; i < tokenizer.count() - 1; ++i)
 	{
 		std::vector<int> indexes;
 		std::string name = tokenizer[i];
@@ -241,7 +245,6 @@ JSON::Object::Ptr JSONConfiguration::findStart(const std::string& key, std::stri
 
 void JSONConfiguration::setValue(const std::string& key, const Poco::DynamicAny& value)
 {
-
 	std::string sValue;
 
 	value.convert<std::string>(sValue);
@@ -276,12 +279,12 @@ void JSONConfiguration::setValue(const std::string& key, const Poco::DynamicAny&
 		}
 
 		JSON::Array::Ptr arr = result.extract<JSON::Array::Ptr>();
-		for(std::vector<int>::iterator it = indexes.begin(); it != indexes.end() - 1; ++it)
+		for (std::vector<int>::iterator it = indexes.begin(); it != indexes.end() - 1; ++it)
 		{
 			JSON::Array::Ptr nextArray = arr->getArray(*it);
 			if (nextArray.isNull())
 			{
-				for(int i = static_cast<int>(arr->size()); i <= *it; ++i)
+				for (int i = static_cast<int>(arr->size()); i <= *it; ++i)
 				{
 					Poco::DynamicAny nullValue;
 					arr->add(nullValue);
@@ -345,14 +348,14 @@ void JSONConfiguration::enumerate(const std::string& key, Keys& range) const
 
 void JSONConfiguration::save(std::ostream& ostr, unsigned int indent) const
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	_object->stringify(ostr, indent);
 }
 
 
 void JSONConfiguration::removeRaw(const std::string& key)
-
 {
-
 	std::string lastPart;
 	JSON::Object::Ptr parentObject = findStart(key, lastPart);
 	std::vector<int> indexes;
@@ -367,7 +370,6 @@ void JSONConfiguration::removeRaw(const std::string& key)
 		DynamicAny result = parentObject->get(lastPart);
 		if (!result.isEmpty() && result.type() == typeid(JSON::Array::Ptr))
 		{
-
 			JSON::Array::Ptr arr = result.extract<JSON::Array::Ptr>();
 			for(std::vector<int>::iterator it = indexes.begin(); it != indexes.end() - 1; ++it)
 			{
@@ -376,7 +378,6 @@ void JSONConfiguration::removeRaw(const std::string& key)
 			arr->remove(indexes.back());
 		}
 	}
-
 }
 
 

Util/src/LayeredConfiguration.cpp

@@ -73,6 +73,8 @@ void LayeredConfiguration::add(AbstractConfiguration::Ptr pConfig, int priority,
 
 void LayeredConfiguration::add(AbstractConfiguration::Ptr pConfig, const std::string& label, int priority, bool writeable)
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	ConfigItem item;
 	item.pConfig   = pConfig;
 	item.priority  = priority;
@@ -87,6 +89,8 @@ void LayeredConfiguration::add(AbstractConfiguration::Ptr pConfig, const std::st
 
 void LayeredConfiguration::removeConfiguration(AbstractConfiguration::Ptr pConfig)
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	for (ConfigList::iterator it = _configs.begin(); it != _configs.end(); ++it)
 	{
 		if (it->pConfig == pConfig)
@@ -100,6 +104,8 @@ void LayeredConfiguration::removeConfiguration(AbstractConfiguration::Ptr pConfi
 
 AbstractConfiguration::Ptr LayeredConfiguration::find(const std::string& label) const
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	for (const auto& conf: _configs)
 	{
 		if (conf.label == label) return conf.pConfig;

Util/src/MapConfiguration.cpp

@@ -32,6 +32,8 @@ MapConfiguration::~MapConfiguration()
 
 void MapConfiguration::copyTo(AbstractConfiguration& config)
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	for (const auto& p: _map)
 	{
 		config.setString(p.first, p.second);
@@ -41,6 +43,8 @@ void MapConfiguration::copyTo(AbstractConfiguration& config)
 
 void MapConfiguration::clear()
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+	
 	_map.clear();
 }
 

Util/src/PropertyFileConfiguration.cpp

@@ -53,6 +53,8 @@ PropertyFileConfiguration::~PropertyFileConfiguration()
 
 void PropertyFileConfiguration::load(std::istream& istr)
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	clear();
 	while (!istr.eof())
 	{
@@ -73,6 +75,8 @@ void PropertyFileConfiguration::load(const std::string& path)
 
 void PropertyFileConfiguration::save(std::ostream& ostr) const
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	MapConfiguration::iterator it = begin();
 	MapConfiguration::iterator ed = end();
 	while (it != ed)

Util/src/XMLConfiguration.cpp

@@ -159,6 +159,8 @@ void XMLConfiguration::load(const Poco::XML::Document* pDocument)
 {
 	poco_check_ptr (pDocument);
 
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	_pDocument = Poco::XML::AutoPtr<Poco::XML::Document>(const_cast<Poco::XML::Document*>(pDocument), true);
 	_pRoot     = Poco::XML::AutoPtr<Poco::XML::Node>(pDocument->documentElement(), true);
 }
@@ -174,6 +176,8 @@ void XMLConfiguration::load(const Poco::XML::Node* pNode)
 	}
 	else
 	{
+		AbstractConfiguration::ScopedLock lock(*this);
+
 		_pDocument = Poco::XML::AutoPtr<Poco::XML::Document>(pNode->ownerDocument(), true);
 		_pRoot     = Poco::XML::AutoPtr<Poco::XML::Node>(const_cast<Poco::XML::Node*>(pNode), true);
 	}
@@ -182,6 +186,8 @@ void XMLConfiguration::load(const Poco::XML::Node* pNode)
 
 void XMLConfiguration::loadEmpty(const std::string& rootElementName)
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	_pDocument = new Poco::XML::Document;
 	_pRoot     = _pDocument->createElement(rootElementName);
 	_pDocument->appendChild(_pRoot);
@@ -190,6 +196,8 @@ void XMLConfiguration::loadEmpty(const std::string& rootElementName)
 
 void XMLConfiguration::save(const std::string& path) const
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	Poco::XML::DOMWriter writer;
 	writer.setNewLine("\n");
 	writer.setOptions(Poco::XML::XMLWriter::PRETTY_PRINT);
@@ -199,6 +207,8 @@ void XMLConfiguration::save(const std::string& path) const
 
 void XMLConfiguration::save(std::ostream& ostr) const
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	Poco::XML::DOMWriter writer;
 	writer.setNewLine("\n");
 	writer.setOptions(Poco::XML::XMLWriter::PRETTY_PRINT);
@@ -208,12 +218,16 @@ void XMLConfiguration::save(std::ostream& ostr) const
 
 void XMLConfiguration::save(Poco::XML::DOMWriter& writer, const std::string& path) const
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	writer.writeNode(path, _pDocument);
 }
 
 
 void XMLConfiguration::save(Poco::XML::DOMWriter& writer, std::ostream& ostr) const
 {
+	AbstractConfiguration::ScopedLock lock(*this);
+
 	writer.writeNode(ostr, _pDocument);
 }
 
@@ -476,4 +490,5 @@ Poco::XML::Node* XMLConfiguration::findAttribute(const std::string& name, Poco::
 
 } } // namespace Poco::Util
 
+
 #endif // POCO_UTIL_NO_XMLCONFIGURATION