From 8df6cd065b63ae0c3f6e25685580afaeaac2348d Mon Sep 17 00:00:00 2001 From: "Zane U. Ji" Date: Mon, 27 Aug 2012 20:25:08 +0800 Subject: [PATCH] Fixed a few memory leaks --- src/threadreaper.cpp | 61 ++++++++++++++++++++++++++++++++++++++ src/threadreaper.h | 42 ++++++++++++++++++++++++++ src/validationthread.cpp | 28 +++++++++++------ src/validationthread.h | 11 ++++++- src/wrapxerces.cpp | 17 +++++++++-- src/wrapxerces.h | 3 +- src/xmlcopyeditor.cpp | 15 +++++----- src/xmlctrl.cpp | 3 +- src/xmlpromptgenerator.cpp | 15 ++++++---- 9 files changed, 166 insertions(+), 29 deletions(-) create mode 100644 src/threadreaper.cpp create mode 100644 src/threadreaper.h diff --git a/src/threadreaper.cpp b/src/threadreaper.cpp new file mode 100644 index 0000000..1298642 --- /dev/null +++ b/src/threadreaper.cpp @@ -0,0 +1,61 @@ +/* + * Copyright 2012 Zane U. Ji. + * + * This file is part of Xml Copy Editor. + * + * Xml Copy Editor is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * Xml Copy Editor is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Xml Copy Editor; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "threadreaper.h" + +extern wxCriticalSection xmlcopyeditorCriticalSection; + +ThreadReaper::ThreadReaper () +{ +} + +ThreadReaper::~ThreadReaper () +{ + //wxCriticalSectionLocker lock ( xmlcopyeditorCriticalSection ); + + std::vector >::iterator itr; + for ( itr = mList.begin(); itr != mList.end(); itr++) + { + (**itr).Kill(); + } +} + +ThreadReaper &ThreadReaper::get() +{ + static ThreadReaper reaper; + return reaper; +} + +void ThreadReaper::add ( wxThread *thread ) +{ + // Make sure everything is valid when wxPostMessage is called + // and protect mList + wxCriticalSectionLocker lock ( xmlcopyeditorCriticalSection ); + + mList.push_back ( boost::shared_ptr ( thread ) ); + + std::vector >::iterator itr; + for ( itr = mList.begin(); itr != mList.end(); ) + { + if ( (**itr).IsAlive() ) + itr++; + else + itr = mList.erase ( itr ); + } +} diff --git a/src/threadreaper.h b/src/threadreaper.h new file mode 100644 index 0000000..2d5c35a --- /dev/null +++ b/src/threadreaper.h @@ -0,0 +1,42 @@ +/* + * Copyright 2012 Zane U. Ji. + * + * This file is part of Xml Copy Editor. + * + * Xml Copy Editor is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * Xml Copy Editor is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Xml Copy Editor; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef THREADREAPER_H_ +#define THREADREAPER_H_ + +#include +#include +#include + +class ThreadReaper +{ +protected: + ThreadReaper(); + virtual ~ThreadReaper(); + +public: + static ThreadReaper &get(); + + void add ( wxThread *thread ); + +protected: + std::vector > mList; +}; + +#endif /* THREADREAPER_H_ */ diff --git a/src/validationthread.cpp b/src/validationthread.cpp index b6a370c..24ad894 100644 --- a/src/validationthread.cpp +++ b/src/validationthread.cpp @@ -4,7 +4,9 @@ #include "wrapxerces.h" #include #include +#include "threadreaper.h" +extern wxCriticalSection xmlcopyeditorCriticalSection; DEFINE_EVENT_TYPE(wxEVT_COMMAND_VALIDATION_COMPLETED); @@ -15,6 +17,7 @@ ValidationThread::ValidationThread ( const char *catalogPath, const char *catalogUtilityPath ) : wxThread ( wxTHREAD_JOINABLE ) + , mStopping ( false ) { if ( buffer == NULL ) { @@ -35,25 +38,22 @@ void *ValidationThread::Entry() myCatalogPath, myCatalogUtilityPath ) ); + if ( TestDestroy() ) { - //wxCriticalSectionLocker locker ( xmlcopyeditorCriticalSection ); - if ( TestDestroy() ) - { - return NULL; - } + return NULL; } myIsSucceeded = validator->validateMemory ( myBuffer.c_str(), + myBuffer.size(), mySystem.c_str(), - myBuffer.size() ); + this ); if ( TestDestroy() ) { return NULL; } - extern wxCriticalSection xmlcopyeditorCriticalSection; wxCriticalSectionLocker locker ( xmlcopyeditorCriticalSection ); if ( myIsSucceeded ) @@ -67,8 +67,18 @@ void *ValidationThread::Entry() myMessage = validator->getLastError(); } - wxCommandEvent event ( wxEVT_COMMAND_VALIDATION_COMPLETED ); - wxPostEvent ( myEventHandler, event ); + if ( !TestDestroy() ) + { + wxCommandEvent event ( wxEVT_COMMAND_VALIDATION_COMPLETED ); + wxPostEvent ( myEventHandler, event ); + } return NULL; } + +void ValidationThread::PendingDelete () +{ + Cancel(); + + ThreadReaper::get().add ( this ); +} diff --git a/src/validationthread.h b/src/validationthread.h index 2d0adf1..2e51ec2 100644 --- a/src/validationthread.h +++ b/src/validationthread.h @@ -22,12 +22,21 @@ public: bool isSucceeded () { return myIsSucceeded; } const std::pair &getPosition() { return myPosition; } const wxString &getMessage() { return myMessage; } -private: + + void PendingDelete(); + // Since we can't call wxThread::m_internal->Cancel(), the original + // TestDestroy() is useless. Here is the work around. + virtual void Cancel() { mStopping = true; } + virtual bool TestDestroy() { return mStopping || wxThread::TestDestroy(); } + +protected: wxEvtHandler *myEventHandler; std::string myBuffer, mySystem, myCatalogPath, myCatalogUtilityPath; bool myIsSucceeded; std::pair myPosition; wxString myMessage; + + bool mStopping; }; #endif diff --git a/src/wrapxerces.cpp b/src/wrapxerces.cpp index d578ffd..cac7687 100755 --- a/src/wrapxerces.cpp +++ b/src/wrapxerces.cpp @@ -113,8 +113,9 @@ bool WrapXerces::validate ( const std::string& fileName ) // tbd: cache grammar bool WrapXerces::validateMemory ( const char *buffer, + size_t len, const char *system, - unsigned len ) + wxThread *thread /*= NULL*/ ) { std::auto_ptr parser ( XMLReaderFactory::createXMLReader() ); @@ -125,7 +126,7 @@ bool WrapXerces::validateMemory ( //parser->setFeature ( XMLUni::fgXercesSchemaFullChecking, true ); parser->setFeature ( XMLUni::fgXercesValidationErrorAsFatal, true ); parser->setFeature ( XMLUni::fgXercesLoadExternalDTD, true ); - + DefaultHandler handler; MySAX2Handler mySAX2Handler; parser->setContentHandler ( &handler ); @@ -138,7 +139,17 @@ bool WrapXerces::validateMemory ( try { - parser->parse ( source ); + if ( thread == NULL ) + { + parser->parse ( source ); + } + else if ( !thread->TestDestroy() ) + { + XMLPScanToken token; + if ( parser->parseFirst ( source, token ) ) + while ( (!thread->TestDestroy()) && parser->parseNext ( token ) ) + continue; + } } catch ( XMLException& e ) { diff --git a/src/wrapxerces.h b/src/wrapxerces.h index 6de1b55..9a164a0 100755 --- a/src/wrapxerces.h +++ b/src/wrapxerces.h @@ -41,7 +41,8 @@ class WrapXerces std::string catalogUtilityPath = "" ); ~WrapXerces(); bool validate ( const std::string& fileName ); - bool validateMemory ( const char *buffer, const char *system, unsigned len ); + bool validateMemory ( const char *buffer, size_t len, + const char *system, wxThread *thread = NULL ); const wxString &getLastError(); std::pair getErrorPosition(); static wxString toString ( const XMLCh *str ); diff --git a/src/xmlcopyeditor.cpp b/src/xmlcopyeditor.cpp index 4a0b60f..7b8e4a0 100755 --- a/src/xmlcopyeditor.cpp +++ b/src/xmlcopyeditor.cpp @@ -3123,7 +3123,7 @@ bool MyFrame::openFile ( wxString& fileName, bool largeFile ) statusProgress ( wxEmptyString ); - char *iconvBuffer = 0; + wxCharBuffer iconvBuffer; size_t iconvBufferLen = 0; char *finalBuffer; @@ -3230,21 +3230,21 @@ bool MyFrame::openFile ( wxString& fileName, bool largeFile ) iconvLenMultiplier = 1; } + size_t nconv; + char *buffer; size_t iconvBufferLeft, docBufferLeft; iconvBufferLen = iconvBufferLeft = (docBufferLen - nBOM) * iconvLenMultiplier + 1; docBufferLeft = docBufferLen; - iconvBuffer = new char[iconvBufferLen]; - finalBuffer = iconvBuffer; // iconvBuffer will be incremented by iconv - size_t nconv; - + iconvBuffer.extend ( iconvBufferLen ); + finalBuffer = buffer = iconvBuffer.data(); // buffer will be incremented by iconv nconv = reinterpret_cast < universal_iconv & > ( iconv ) ( cd, &docBuffer, &docBufferLeft, - &iconvBuffer, + &buffer, &iconvBufferLeft ); - *iconvBuffer = '\0'; + *buffer = '\0'; iconv_close ( cd ); @@ -3255,7 +3255,6 @@ bool MyFrame::openFile ( wxString& fileName, bool largeFile ) fileName.c_str(), wideEncoding.c_str() ); messagePane ( message, CONST_STOP ); - delete[] finalBuffer; delete binaryfile; //delete memorymap; return false; } diff --git a/src/xmlctrl.cpp b/src/xmlctrl.cpp index 0bc8c4f..695b3dd 100755 --- a/src/xmlctrl.cpp +++ b/src/xmlctrl.cpp @@ -138,8 +138,7 @@ XmlCtrl::~XmlCtrl() if ( validationThread != NULL ) { - validationThread->Kill(); - delete validationThread; + validationThread->PendingDelete(); } } diff --git a/src/xmlpromptgenerator.cpp b/src/xmlpromptgenerator.cpp index 6758a01..13e2ef9 100755 --- a/src/xmlpromptgenerator.cpp +++ b/src/xmlpromptgenerator.cpp @@ -299,6 +299,7 @@ int XMLCALL XmlPromptGenerator::externalentityrefhandler ( PromptGeneratorData *d; d = ( PromptGeneratorData * ) p; // arg is set to user data in c'tor + int ret; std::string buffer; // auxPath req'd? @@ -307,15 +308,17 @@ int XMLCALL XmlPromptGenerator::externalentityrefhandler ( ReadFile::run ( d->auxPath, buffer ); if ( buffer.empty() ) { - return false; + return XML_STATUS_ERROR; } std::string encoding = XmlEncodingHandler::get ( buffer ); XML_Parser dtdParser = XML_ExternalEntityParserCreate ( d->p, context, encoding.c_str() ); if ( !dtdParser ) - return false; + return XML_STATUS_ERROR; XML_SetBase ( dtdParser, d->auxPath.c_str() ); - return XML_Parse ( dtdParser, buffer.c_str(), buffer.size(), true ); + ret = XML_Parse ( dtdParser, buffer.c_str(), buffer.size(), true ); + XML_ParserFree ( dtdParser ); + return ret; } std::string stdPublicId; @@ -358,7 +361,7 @@ int XMLCALL XmlPromptGenerator::externalentityrefhandler ( std::string encoding = XmlEncodingHandler::get ( buffer ); XML_Parser dtdParser = XML_ExternalEntityParserCreate ( d->p, context, encoding.c_str() ); if ( !dtdParser ) - return false; + return XML_STATUS_ERROR; wxString wideName, wideDir; wideName = wxString ( stdSystemId.c_str(), wxConvUTF8, stdSystemId.size() ); @@ -366,7 +369,9 @@ int XMLCALL XmlPromptGenerator::externalentityrefhandler ( wideDir = fn.GetPath(); XML_SetBase ( dtdParser, wideName.mb_str ( wxConvUTF8 ) ); - return XML_Parse ( dtdParser, buffer.c_str(), buffer.size(), true ); + ret = XML_Parse ( dtdParser, buffer.c_str(), buffer.size(), true ); + XML_ParserFree ( dtdParser ); + return ret; } void XMLCALL XmlPromptGenerator::entitydeclhandler (