Skip to content

Commit

Permalink
xmlutil::Document employes mutexes now to lock data
Browse files Browse the repository at this point in the history
This is very unlikely to be waterproof, since it's still possible to
hold stale xmlutil::Node objects, but it seems to prevent the crashes
during startup at least.
  • Loading branch information
codereader committed Dec 26, 2016
1 parent b02f6c2 commit 73dfcc1
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 17 deletions.
42 changes: 31 additions & 11 deletions libs/xmlutil/Document.cpp
Expand Up @@ -21,8 +21,10 @@ Document::Document(const Document& other) :
_xmlDoc(other._xmlDoc)
{}

Document::~Document() {
if (_xmlDoc != NULL) {
Document::~Document()
{
if (_xmlDoc != nullptr)
{
// Free the xml document memory
xmlFreeDoc(_xmlDoc);
}
Expand All @@ -40,7 +42,10 @@ Document Document::create()
return Document(doc);
}

void Document::addTopLevelNode(const std::string& name) {
void Document::addTopLevelNode(const std::string& name)
{
std::lock_guard<std::mutex> lock(_lock);

if (!isValid()) {
return; // is not Valid, place an assertion here?
}
Expand All @@ -62,8 +67,10 @@ void Document::addTopLevelNode(const std::string& name) {
xmlFree(emptyStr);
}

Node Document::getTopLevelNode() const {
if (!isValid()) {
Node Document::getTopLevelNode() const
{
if (!isValid())
{
// Invalid Document, return a NULL node
return Node(NULL);
}
Expand All @@ -73,6 +80,8 @@ Node Document::getTopLevelNode() const {

void Document::importDocument(Document& other, Node& importNode)
{
std::lock_guard<std::mutex> lock(_lock);

// Locate the top-level node(s) of the other document
xml::NodeList topLevelNodes = other.findXPath("/*");

Expand Down Expand Up @@ -102,27 +111,35 @@ void Document::importDocument(Document& other, Node& importNode)
}
}

void Document::copyNodes(const NodeList& nodeList) {
if (!isValid() || _xmlDoc->children == NULL) {
void Document::copyNodes(const NodeList& nodeList)
{
std::lock_guard<std::mutex> lock(_lock);

if (!isValid() || _xmlDoc->children == NULL)
{
return; // is not Valid, place an assertion here?
}

// Copy the child nodes one by one
for (std::size_t i = 0; i < nodeList.size(); i++) {
for (std::size_t i = 0; i < nodeList.size(); i++)
{
// Copy the node
xmlNodePtr node = xmlCopyNode(nodeList[i].getNodePtr(), 1);
// Add this node to the top level node of this document
xmlAddChild(xmlDocGetRootElement(_xmlDoc), node);
}
}

bool Document::isValid() const {
return _xmlDoc != NULL;
bool Document::isValid() const
{
return _xmlDoc != nullptr;
}

// Evaluate an XPath expression and return matching Nodes.
NodeList Document::findXPath(const std::string& path) const
{
std::lock_guard<std::mutex> lock(_lock);

// Set up the XPath context
xmlXPathContextPtr context = xmlXPathNewContext(_xmlDoc);

Expand Down Expand Up @@ -158,7 +175,10 @@ NodeList Document::findXPath(const std::string& path) const
}

// Saves the file to the disk via xmlSaveFormatFile
void Document::saveToFile(const std::string& filename) const {
void Document::saveToFile(const std::string& filename) const
{
std::lock_guard<std::mutex> lock(_lock);

xmlSaveFormatFile(filename.c_str(), _xmlDoc, 1);
}

Expand Down
12 changes: 6 additions & 6 deletions libs/xmlutil/Document.h
@@ -1,8 +1,9 @@
#ifndef DOCUMENT_H_
#define DOCUMENT_H_
#pragma once

#include "Node.h"

#include <mutex>

typedef struct _xmlDoc xmlDoc;
typedef xmlDoc *xmlDocPtr;

Expand All @@ -23,10 +24,11 @@ namespace xml
class Document
{
private:

// Contained xmlDocPtr.
// Contained xmlDocPtr.
xmlDocPtr _xmlDoc;

mutable std::mutex _lock;

public:
// Construct a Document using the provided xmlDocPtr.
Document(xmlDocPtr doc);
Expand Down Expand Up @@ -70,5 +72,3 @@ class Document
};

}

#endif /*DOCUMENT_H_*/

0 comments on commit 73dfcc1

Please sign in to comment.