Fixed multi-thread problems with LibXML and Xerces-C++

src/validationthread.cpp

@@ -5,31 +5,28 @@
 #include <stdexcept>
 #include <memory>
 
+
+wxDEFINE_EVENT(wxEVT_COMMAND_VALIDATION_COMPLETED, wxThreadEvent);
+
 ValidationThread::ValidationThread (
+	wxEvtHandler *handler,
 	const char *buffer,
 	const char *system,
 	const char *catalogPath,
-	const char *catalogUtilityPath,
+	const char *catalogUtilityPath )
-	bool *finished,
+	: wxThread ( wxTHREAD_JOINABLE )
-	bool *success,
-	bool *release,
-	std::pair<int, int> *position,
-	wxString *message ) : wxThread()
 {
-	if (!buffer || !success || !position || !message )
+	if ( buffer == NULL )
 	{
 		throw;
 	}
 
+	myEventHandler = handler;
 	myBuffer = buffer;
 	mySystem = system;
 	myCatalogPath = catalogPath;
 	myCatalogUtilityPath = catalogUtilityPath;
-	myFinishedPtr = finished;
+	myIsSucceeded = false;
-	mySuccessPtr = success;
-	myReleasePtr = release;
-	myPositionPtr = position;
-	myMessagePtr = message;
 }
 
 void *ValidationThread::Entry()
@@ -40,47 +37,38 @@ void *ValidationThread::Entry()
 	
 	{
 		//wxCriticalSectionLocker locker ( xmlcopyeditorCriticalSection );
-		if ( *myReleasePtr || TestDestroy()  )
+		if ( TestDestroy()  )
 		{
-			Exit();
 			return NULL;
 		}
 	}
-	
+
-	bool res = validator->validateMemory (
+	myIsSucceeded = validator->validateMemory (
 		myBuffer.c_str(),
 		mySystem.c_str(),
 		myBuffer.size() );
+
+	if ( TestDestroy() )
 	{
-		//wxCriticalSectionLocker locker ( xmlcopyeditorCriticalSection );
+		return NULL;
-		if ( *myReleasePtr  || TestDestroy() )
-		{
-			Exit();
-			return NULL;
-		}
-	
-		if ( !res )
-		{
-			*mySuccessPtr = res;
-			*myPositionPtr = validator->getErrorPosition();
-			*myMessagePtr = validator->getLastError();		
-		}
-		else
-		{
-			*mySuccessPtr = true;
-			*myPositionPtr = std::make_pair ( 0, 0 );
-			*myMessagePtr = wxEmptyString;
-		}
 	}
+
+	extern wxCriticalSection xmlcopyeditorCriticalSection;
+	wxCriticalSectionLocker locker ( xmlcopyeditorCriticalSection );
+
+	if ( myIsSucceeded )
+	{
+		myPosition = std::make_pair ( 0, 0 );
+		myMessage = wxEmptyString;
+	}
+	else
+	{
+		myPosition = validator->getErrorPosition();
+		myMessage = validator->getLastError();
+	}
+
+	wxEvent *event = new wxThreadEvent(wxEVT_THREAD, wxEVT_COMMAND_VALIDATION_COMPLETED);
+	wxQueueEvent ( myEventHandler, event );
+
 	return NULL;
 }
-
-void ValidationThread::OnExit()
-{
-	{
-		//wxCriticalSectionLocker locker ( xmlcopyeditorCriticalSection );
-		if ( *myReleasePtr )
-			return;
-		*myFinishedPtr = true;
-	}
-}

src/validationthread.h

@@ -6,25 +6,28 @@
 #include <string>
 #include <wx/thread.h>
 
+wxDECLARE_EVENT(wxEVT_COMMAND_VALIDATION_COMPLETED, wxThreadEvent);
+
 class ValidationThread : public wxThread
 {
 public:
 	ValidationThread (
-                     const char *buffer,
+	                 wxEvtHandler *handler,
-                     const char *system,
+	                 const char *buffer,
-                     const char *catalogPath,
+	                 const char *system,
-                     const char *catalogUtilityPath,
+	                 const char *catalogPath,
-                     bool *finished,
+	                 const char *catalogUtilityPath );
-                     bool *success,
-                     bool *release, std::pair<int, int> *position,
-                     wxString *message );
 	virtual void *Entry();
-	virtual void OnExit();
+	void setBuffer ( const char *buffer, const char *system );
+	bool isSucceeded () { return myIsSucceeded; }
+	const std::pair<int, int> &getPosition() { return myPosition; }
+	const wxString &getMessage() { return myMessage; }
 private:
+	wxEvtHandler *myEventHandler;
 	std::string myBuffer, mySystem, myCatalogPath, myCatalogUtilityPath;
-	bool *myFinishedPtr, *mySuccessPtr, *myReleasePtr;
+	bool myIsSucceeded;
-	std::pair<int, int> *myPositionPtr;
+	std::pair<int, int> myPosition;
-	wxString *myMessagePtr;
+	wxString myMessage;
 };
 
 #endif

src/wraplibxml.cpp

@@ -27,18 +27,67 @@
 
 #include <wx/wx.h>
 
+void WrapLibxml::Init() throw()
+{
+	static class Initializer
+	{
+	public:
+		Initializer () throw ()
+		{
+			xmlSetGenericErrorFunc ( xmlGenericErrorContext, &Initializer::OnXmlGenericError );
+
+			LIBXML_TEST_VERSION
+			xmlInitializeCatalog();
+
+			initGenericErrorDefaultFunc ( NULL );
+		}
+		~Initializer ()
+		{
+			xsltCleanupGlobals();
+			xmlCatalogCleanup();
+			xmlCleanupParser();
+		}
+
+		static void XMLCDECL OnXmlGenericError (void *ctx, const char *msg, ...) throw()
+		{
+			va_list args;
+
+			size_t size = 128;
+			std::string buffer;
+			int chars;
+			for (;;)
+			{
+				buffer.resize ( size );
+				if ( buffer.size() < size )
+					throw std::runtime_error ( "Out of memory" );
+
+				va_start(args, msg);
+				chars = vsnprintf( (char *) buffer.c_str(), size, msg, args);
+				va_end(args);
+
+				if ( chars >= 0 && ( size_t ) chars < size )
+				{
+					buffer.resize ( chars );
+					throw std::runtime_error ( buffer );
+				}
+				if ( chars >= 0 )
+					size = chars + 1;
+				else
+					throw std::runtime_error (
+					    std::string ( "Can't format message: " ) + msg );
+			}
+		}
+	} dummy;
+}
+
 WrapLibxml::WrapLibxml ( bool netAccessParameter, const std::string& catalogPathParameter )
 		: netAccess ( netAccessParameter ), catalogPath ( catalogPathParameter )
 {
-	LIBXML_TEST_VERSION
+	WrapLibxml::Init();
-	xmlInitializeCatalog();
 }
 
 WrapLibxml::~WrapLibxml()
 {
-	xsltCleanupGlobals();
-	xmlCleanupParser();
-	xmlCatalogCleanup();
 }
 
 bool WrapLibxml::validate ( const std::string& fileName )
@@ -82,8 +131,6 @@ bool WrapLibxml::validateRelaxNG (
 {
 	output = "";
 
-	xmlCleanupParser();
-
 	xmlRelaxNGValidCtxtPtr ctxtPtr;
 	xmlDocPtr docPtr;
 	xmlRelaxNGParserCtxtPtr schemaParserCtxtPtr;
@@ -133,7 +180,6 @@ bool WrapLibxml::validateW3CSchema (
 {
 	output = "";
 
-	xmlCleanupParser();
 	xmlSchemaValidCtxtPtr ctxtPtr;
 	xmlDocPtr docPtr;
 	xmlSchemaParserCtxtPtr schemaParserCtxtPtr;
@@ -184,7 +230,6 @@ bool WrapLibxml::parse (
 {
 	output = "";
 
-	xmlCleanupParser();
 	xmlParserCtxtPtr ctxt;
 	xmlDocPtr docPtr;
 
@@ -245,8 +290,6 @@ bool WrapLibxml::xpath ( const std::string& path, const std::string& fileName )
 {
 	output = "";
 
-	xmlCleanupParser();
-
 	xmlParserCtxtPtr ctxt;
 	xmlDocPtr docPtr;
 
@@ -320,8 +363,6 @@ bool WrapLibxml::xpath ( const std::string& path, const std::string& fileName )
 	xmlFreeDoc ( docPtr );
 	xmlFreeParserCtxt ( ctxt );
 
-	xmlCleanupParser();
-
 	return xpathIsValid;
 }
 
@@ -330,7 +371,6 @@ bool WrapLibxml::xslt (
     const std::string& fileName
 )
 {
-	xmlCleanupParser();
 	output = "";
 
 	xsltStylesheetPtr cur;
@@ -388,8 +428,6 @@ bool WrapLibxml::xslt (
 
 bool WrapLibxml::bufferWellFormed ( const std::string& buffer )
 {
-	xmlCleanupParser();
-
 	xmlParserCtxtPtr ctxt = xmlNewParserCtxt();
 	if ( !ctxt )
 		return false;
@@ -415,8 +453,6 @@ int WrapLibxml::saveEncoding (
     const std::string& fileName,
     const std::string& encoding )
 {
-	xmlCleanupParser();
-
 	xmlParserCtxtPtr ctxt = xmlNewParserCtxt();
 	if ( !ctxt )
 		return -1;
@@ -455,8 +491,6 @@ int WrapLibxml::saveEncodingFromFile (
     const std::string& fileNameDestination,
     const std::string& encoding )
 {
-	xmlCleanupParser();
-
 	xmlParserCtxtPtr ctxt = xmlNewParserCtxt();
 	if ( !ctxt )
 		return -1;

src/wraplibxml.h

@@ -38,6 +38,7 @@
 class WrapLibxml
 {
 	public:
+		static void Init() throw();
 		WrapLibxml (
 		    bool netAccessParameter = false,
 		    const std::string& catalogPathParameter = "catalog" );

src/wrapxerces.cpp

@@ -30,8 +30,26 @@
 
 using namespace xercesc;
 
+void WrapXerces::Init() throw()
+{
+	static class Initializer
+	{
+	public:
+		Initializer()
+		{
+			XMLPlatformUtils::Initialize();
+		}
+		~Initializer()
+		{
+			XMLPlatformUtils::Terminate();
+		}
+	} dummy;
+}
+
 WrapXerces::WrapXerces( std::string catalogPath, std::string catalogUtilityPath )
 {
+	WrapXerces::Init();
+
 	errorPosition = std::make_pair ( 1, 1 );
 	catalogResolver = new XercesCatalogResolver( catalogPath, catalogUtilityPath );
 }
@@ -98,7 +116,7 @@ bool WrapXerces::validateMemory (
 	const char *system,
 	unsigned len )
 {
-	SAX2XMLReader *parser = XMLReaderFactory::createXMLReader();
+	std::auto_ptr<SAX2XMLReader> parser ( XMLReaderFactory::createXMLReader() );
 
 	parser->setFeature ( XMLUni::fgSAX2CoreNameSpaces, true );
 	parser->setFeature ( XMLUni::fgSAX2CoreValidation, true );
@@ -116,24 +134,19 @@ bool WrapXerces::validateMemory (
 	parser->setEntityResolver ( catalogResolver );
 
 	XMLByte* xmlBuffer = (XMLByte*) buffer;
-        MemBufInputSource source (
+	MemBufInputSource source ( xmlBuffer, len, system );
-		xmlBuffer,
+
-                len,
-		system );
-  
 	try
 	{
 		parser->parse ( source );
 	}
 	catch ( XMLException& e )
 	{
-		delete parser;
 		lastError = wxEmptyString;
 		return false;
 	}
 	catch ( SAXParseException& e )
 	{
-		delete parser;
 		lastError << _T ( "Ln " ) << e.getLineNumber() << _T ( " Col " )
 		    << e.getColumnNumber() << _T ( ": " ) << toString ( e.getMessage() );
 		errorPosition = std::make_pair ( e.getLineNumber(), e.getColumnNumber() );
@@ -141,11 +154,9 @@ bool WrapXerces::validateMemory (
 	}
 	catch ( ... )
 	{
-		delete parser;
 		lastError = wxEmptyString;
 		return false;
 	}
-	delete parser;
 	return true;
 }
 

src/wrapxerces.h

@@ -36,6 +36,7 @@ using namespace xercesc;
 class WrapXerces
 {
 	public:
+		static void Init() throw ();
 		WrapXerces( std::string catalogPath = "",
                     std::string catalogUtilityPath = "" );
 		~WrapXerces();

src/xmlcopyeditor.cpp

@@ -324,9 +324,6 @@ MyApp::~MyApp()
 	delete checker;
 	delete server;
 	delete connection;
-
-	// Terminate Xerces-C++
-	XMLPlatformUtils::Terminate();
 }
 
 bool MyApp::OnInit()
@@ -387,8 +384,11 @@ bool MyApp::OnInit()
 		wxImage::AddHandler ( new wxPNGHandler );
 		wxSystemOptions::SetOption ( _T ( "msw.remap" ), 0 );
 
+		// Initialize libxml
+		WrapLibxml::Init();
+
 		// Initialize Xerces-C++
-		XMLPlatformUtils::Initialize();
+		WrapXerces::Init();
 
 		frame = new MyFrame (
 		    _ ( "XML Copy Editor" ),

src/xmlctrl.cpp

@@ -25,6 +25,8 @@
 #include "xmlcopyeditor.h" // needed to enable validation-as-you-type alerts
 #include <utility>
 #include <memory>
+#include "validationthread.h"
+
 
 // adapted from wxSTEdit (c) 2005 John Labenski, Otto Wyss
 #define XMLCTRL_HASBIT(value, bit) (((value) & (bit)) != 0)
@@ -38,6 +40,7 @@ BEGIN_EVENT_TABLE ( XmlCtrl, wxStyledTextCtrl )
 	EVT_LEFT_UP ( XmlCtrl::OnMouseLeftUp )
 	EVT_RIGHT_UP ( XmlCtrl::OnMouseRightUp )
 	EVT_MIDDLE_DOWN ( XmlCtrl::OnMiddleDown )
+	EVT_THREAD(wxEVT_COMMAND_VALIDATION_COMPLETED, XmlCtrl::OnValidationCompleted)
 END_EVENT_TABLE()
 
 // global protection for validation threads
@@ -68,14 +71,10 @@ XmlCtrl::XmlCtrl (
 		auxPath ( auxPathParameter )
 {
 	validationThread = NULL;
-	validationStarted = false;
+
-	validationFinished = false;
 	grammarFound = false;
 	validationRequired = (buffer) ? true : false; // NULL for plain XML template
 
-	validationReleasePtr = new bool;
-	*validationReleasePtr = false;
-	
 	currentMaxLine = 1;
 
 	applyProperties ( propertiesParameter );
@@ -120,10 +119,10 @@ XmlCtrl::XmlCtrl (
 	applyVisibilityState ( visibilityState );
 	lineBackgroundState = BACKGROUND_STATE_NORMAL;
 
-	for ( int i = 0; i < wxSTC_INDIC_MAX; ++i ) 
+	for ( int i = 0; i < wxSTC_INDIC_MAX; ++i )
 		IndicatorSetStyle ( i, wxSTC_INDIC_HIDDEN );
 	IndicatorSetStyle ( 2, wxSTC_INDIC_SQUIGGLE );
-	IndicatorSetForeground ( 0, *wxRED );
+	IndicatorSetForeground ( 2, *wxRED );
 }
 
 
@@ -133,17 +132,11 @@ XmlCtrl::~XmlCtrl()
 	elementMap.clear();
 	entitySet.clear();
 
-	if ( validationStarted && !validationFinished )
+	if ( validationThread != NULL )
 	{
-		*validationReleasePtr = true;
+		validationThread->Kill();
-
+		delete validationThread;
-		// don't delete validationReleasePtr because the thread will check the value before exiting
-		// this means that 1 variable of type bool will be leaked in this case
-		// the alternative is waiting for the validation thread to finish, which can take anything up to several minutes
-		return;
 	}
-	
-	delete validationReleasePtr;
 }
 
 
@@ -169,28 +162,30 @@ void XmlCtrl::OnIdle ( wxIdleEvent& event )
 {
 	if ( properties.number && type != FILE_TYPE_BINARY )
 		adjustNoColumnWidth(); // exits if unchanged
+}
 
-	// poll validation thread output if any
+void XmlCtrl::OnValidationCompleted ( wxThreadEvent &event )
-	
+{
+	wxCriticalSectionLocker locker ( xmlcopyeditorCriticalSection );
+
+	if ( validationThread == NULL )
+		return;
+
+	MyFrame *frame = (MyFrame *)GetGrandParent();
+	clearErrorIndicators ( GetLineCount() );
+	if ( validationThread->isSucceeded() )
 	{
-		wxCriticalSectionLocker locker ( xmlcopyeditorCriticalSection );
+		frame->statusProgress ( wxEmptyString );
-		if (validationStarted && validationFinished)
-		{
-			validationStarted = false;
-			MyFrame *frame = (MyFrame *)GetGrandParent();
-			if ( validationSuccess )
-			{
-				clearErrorIndicators ( GetLineCount() );
-				frame->statusProgress ( wxEmptyString );
-			}
-			else
-			{
-				clearErrorIndicators ( GetLineCount() );
-				setErrorIndicator ( validationPosition.first - 1, 0 );
-				frame->statusProgress ( validationMessage );
-			}
-		}
 	}
+	else
+	{
+		setErrorIndicator ( validationThread->getPosition().first - 1, 0 );
+		frame->statusProgress ( validationThread->getMessage() );
+	}
+
+	validationThread->Wait();
+	delete validationThread;
+	validationThread = NULL;
 }
 
 void XmlCtrl::OnChar ( wxKeyEvent& event )
@@ -1974,38 +1969,31 @@ bool XmlCtrl::backgroundValidate (
 {
 	if ( !validationRequired )
 		return true;
-	
+
 	wxCriticalSectionLocker locker ( xmlcopyeditorCriticalSection );
-	if ( validationStarted && !validationFinished )
+
+	if ( validationThread != NULL )
 	{
-		*validationReleasePtr = true;
 		return true; // wait for next idle cycle call from main app frame
 	}
 	validationRequired = false;
-	
+
-	*validationReleasePtr = false;
 	validationThread = new ValidationThread(
+		GetEventHandler(),
 		buffer,
 		system,
 		catalogPath.c_str(),
-		catalogUtilityPath.c_str(),
+		catalogUtilityPath.c_str()
-		&validationFinished,
-		&validationSuccess,
-		validationReleasePtr,
-		&validationPosition,
-		&validationMessage
 	);
 
-	if ( validationThread->Create() != wxTHREAD_NO_ERROR )
+	if ( validationThread->Create() != wxTHREAD_NO_ERROR
+	    || validationThread->Run() != wxTHREAD_NO_ERROR )
 	{
-		validationStarted = false;
+		delete validationThread;
-		validationFinished = true;
+		validationThread = NULL;
 		return false;
 	}
-	
+
-	validationStarted = true;
-	validationFinished = false;
-	validationThread->Run();
 	return true;
 }
 

src/xmlctrl.h

@@ -27,7 +27,8 @@
 #include <string>
 #include <set>
 #include <map>
-#include "validationthread.h"
+
+class ValidationThread;
 
 struct XmlCtrlProperties
 {
@@ -146,15 +147,8 @@ class XmlCtrl: public wxStyledTextCtrl
 		bool getValidationRequired();
 		void setValidationRequired ( bool b );
 	private:
-		// the following are used for background validation
+		ValidationThread *validationThread; // used for background validation
-		ValidationThread *validationThread;
+
-		bool validationStarted,
-			validationFinished,
-			validationSuccess;
-		bool *validationReleasePtr;
-		std::pair<int, int> validationPosition;
-		wxString validationMessage;
-	
 		int type;
 		bool *protectTags;
 		bool validationRequired, grammarFound;
@@ -191,6 +185,7 @@ class XmlCtrl: public wxStyledTextCtrl
 		void OnMarginClick ( wxStyledTextEvent& event );
 		void OnChar ( wxKeyEvent& event );
 		void OnIdle ( wxIdleEvent& event );
+		void OnValidationCompleted (wxThreadEvent &event);
 		void OnKeyPressed ( wxKeyEvent& event );
 		void OnMouseLeftDown ( wxMouseEvent& event );
 		void OnMouseLeftUp ( wxMouseEvent& event );