From a26c361d3382f2d7bd3ba5ed4ec62f0cc2331c44 Mon Sep 17 00:00:00 2001 From: Phil Cowans Date: Fri, 16 Mar 2007 09:36:00 +0000 Subject: [PATCH] Fixing memory leaks in conversion mode * Fixing memory leaks in conversion mode svn path=/trunk/; revision=3060 --- Src/DasherCore/CannaConversionHelper.cpp | 35 +++++++++------ Src/DasherCore/ConversionManager.cpp | 70 +++++++++++++++++++---------- Src/DasherCore/ConversionManager.h | 7 ++- Src/DasherCore/ConversionManagerFactory.cpp | 6 ++- Src/DasherCore/DasherNode.cpp | 10 ++++- Src/DasherCore/Makefile.am | 1 + Src/DasherCore/SCENode.cpp | 33 ++++++++++++++ Src/DasherCore/SCENode.h | 45 +++++++++++++++---- Src/Gtk2/Canvas.cpp | 2 +- 9 files changed, 160 insertions(+), 49 deletions(-) create mode 100644 Src/DasherCore/SCENode.cpp diff --git a/Src/DasherCore/CannaConversionHelper.cpp b/Src/DasherCore/CannaConversionHelper.cpp index 2e232406..11a1a714 100644 --- a/Src/DasherCore/CannaConversionHelper.cpp +++ b/Src/DasherCore/CannaConversionHelper.cpp @@ -96,12 +96,15 @@ bool CCannaConversionHelper::Convert(const std::string &strSource, SCENode ** pR } SCENode *pDummyRoot(new SCENode); - pDummyRoot->pChild = NULL; + // pDummyRoot->pChild = NULL; /* Convert each phrase into Kanji */ cd = iconv_open("UTF8", "EUC-JP"); for(int i = nbun-1; i >= 0; --i) { - SCENode *pTail = pDummyRoot->pChild; + SCENode *pTail = pDummyRoot->GetChild(); + + if(pTail) + pTail->Ref(); RkGoTo(context_id, i); // Move to a specific phrase int len = RkGetKanjiList(context_id, buf, BUFSIZE); // Get a list of Kanji candidates @@ -134,6 +137,9 @@ bool CCannaConversionHelper::Convert(const std::string &strSource, SCENode ** pR for(std::vector::reverse_iterator it(vCandidates.rbegin()); it != vCandidates.rend(); ++it) { ProcessCandidate(*it, pDummyRoot, pTail); } + + if(pTail) + pTail->Unref(); } RkEndBun(context_id, 0); // Close phrase division @@ -141,9 +147,10 @@ bool CCannaConversionHelper::Convert(const std::string &strSource, SCENode ** pR free(buf); free(str_utf8); - *pRoot = pDummyRoot->pChild; - - delete pDummyRoot; + *pRoot = pDummyRoot->GetChild(); + + (*pRoot)->Ref(); + pDummyRoot->Unref(); return true; } @@ -178,26 +185,26 @@ void CCannaConversionHelper::ProcessCandidate(std::string strCandidate, SCENode iIdx += iLength; - SCENode *pCurrentChild(pCurrentNode->pChild); // TODO: Initialise + SCENode *pCurrentChild(pCurrentNode->GetChild()); // TODO: Initialise while(pCurrentChild) { if(strSymbol == pCurrentChild->pszConversion) break; - pCurrentChild = pCurrentChild->pNext; + pCurrentChild = pCurrentChild->GetNext(); } if(!pCurrentChild) { // Need a new child pCurrentChild = new SCENode; - pCurrentChild->pNext = pCurrentNode->pChild; - if(iIdx >= strCandidate.size()) - pCurrentChild->pChild = pTail; - else - pCurrentChild->pChild = NULL; + if(pCurrentNode->GetChild()) + pCurrentChild->SetNext(pCurrentNode->GetChild()); + if(pTail && (iIdx >= strCandidate.size())) + pCurrentChild->SetChild(pTail); pCurrentChild->pszConversion = new char[strSymbol.size() + 1]; strcpy(pCurrentChild->pszConversion, strSymbol.c_str()); - pCurrentNode->pChild = pCurrentChild; + pCurrentNode->SetChild(pCurrentChild); + pCurrentChild->Unref(); } pCurrentNode = pCurrentChild; @@ -220,6 +227,6 @@ void CCannaConversionHelper::AssignSizes(SCENode *pStart, Dasher::CLanguageModel iCheck += pNode->NodeSize; --iRemaining; - pNode = pNode->pNext; + pNode = pNode->GetNext(); } } diff --git a/Src/DasherCore/ConversionManager.cpp b/Src/DasherCore/ConversionManager.cpp index f3cb56b7..aa81916d 100644 --- a/Src/DasherCore/ConversionManager.cpp +++ b/Src/DasherCore/ConversionManager.cpp @@ -36,9 +36,11 @@ using namespace Dasher; CConversionManager::CConversionManager(CNodeCreationManager *pNCManager, CConversionHelper *pHelper, CAlphabet *pAlphabet, int CMid) : CNodeManager(2) { + m_pNCManager = pNCManager; m_pHelper = pHelper; m_pAlphabet = pAlphabet; + m_pRoot = NULL; //DOESN'T SEEM INTRINSIC //and check why pHelper may be empty @@ -50,7 +52,7 @@ CConversionManager::CConversionManager(CNodeCreationManager *pNCManager, CConver if(m_pLanguageModel) m_iLearnContext = m_pLanguageModel->CreateEmptyContext(); - m_iRefCount = 1; // TODO: Is this the right way to handle this, or should we initilise to 0 and enforce a reference from the creator? + m_iRefCount = 1; m_iCMID = CMid; // m_iHZCount = 0; @@ -58,8 +60,12 @@ CConversionManager::CConversionManager(CNodeCreationManager *pNCManager, CConver } CConversionManager::~CConversionManager(){ + // for (int i(0);ipNext; + pChild = pChild->GetNext(); ++iNChildren; } @@ -315,7 +321,7 @@ void CConversionManager::PopulateChildren( CDasherNode *pNode ) { SCENode *pCurrentSCEChild; if(pCurrentDataNode->pSCENode) - pCurrentSCEChild = pCurrentDataNode->pSCENode->pChild; + pCurrentSCEChild = pCurrentDataNode->pSCENode->GetChild(); else { if(m_pRoot && !pCurrentDataNode->bType) pCurrentSCEChild = m_pRoot[0]; @@ -403,7 +409,7 @@ void CConversionManager::PopulateChildren( CDasherNode *pNode ) { pNode->Children().push_back(pNewNode); - pCurrentSCEChild = pCurrentSCEChild->pNext; + pCurrentSCEChild = pCurrentSCEChild->GetNext(); ++iIdx; }while(pCurrentSCEChild); @@ -465,7 +471,7 @@ void CConversionManager::PopulateChildren( CDasherNode *pNode ) { } void CConversionManager::ClearNode( CDasherNode *pNode ) { - pNode->m_pNodeManager->Unref(); + // pNode->m_pNodeManager->Unref(); } void CConversionManager::RecursiveDumpTree(SCENode *pCurrent, unsigned int iDepth) { @@ -475,8 +481,8 @@ void CConversionManager::RecursiveDumpTree(SCENode *pCurrent, unsigned int iDept std::cout << " " << pCurrent->pszConversion << " " << pCurrent->IsHeadAndCandNum << " " << pCurrent->CandIndex << " " << pCurrent->IsComplete << " " << pCurrent->AcCharCount << std::endl; - RecursiveDumpTree(pCurrent->pChild, iDepth + 1); - pCurrent = pCurrent->pNext; + RecursiveDumpTree(pCurrent->GetChild(), iDepth + 1); + pCurrent = pCurrent->GetNext(); } } @@ -568,24 +574,42 @@ void CConversionManager::Undo( CDasherNode *pNode ) { } bool CConversionManager::RecursiveDelTree(SCENode* pNode){ + // TODO: Do we actually care about the return value? - SCENode * pTemp; + // TODO: Function now obsolete - if(!pNode) - return 0; - else if(pNode->pChild) - return RecursiveDelTree(pNode->pChild); - else{ + pNode->Unref(); - while(!pNode->pChild){ - pTemp = pNode->pNext; - delete pNode; - pNode = pTemp; - if(!pNode) - return 1; - } - return RecursiveDelTree(pNode->pChild); - } + return false; +// if(!pNode) +// return 0; + +// // Note that this is a lattice, not a tree, so we need to be careful +// // about deleting thing twice + +// if(pNode->pChild) +// RecursiveDeleteTree(pNode->pChild); + +// if(pNode->pNext) +// RecursiveDeleteTree(pNode->pNext); + +// SCENode * pTemp; + +// if(!pNode) +// return 0; +// else if(pNode->pChild) +// return RecursiveDelTree(pNode->pChild); +// else{ + +// while(!pNode->pChild){ +// pTemp = pNode->pNext; +// delete pNode; +// pNode = pTemp; +// if(!pNode) +// return 1; +// } +// return RecursiveDelTree(pNode->pChild); +// } } diff --git a/Src/DasherCore/ConversionManager.h b/Src/DasherCore/ConversionManager.h index 852dd5de..83200d2d 100644 --- a/Src/DasherCore/ConversionManager.h +++ b/Src/DasherCore/ConversionManager.h @@ -81,8 +81,13 @@ namespace Dasher { virtual void Unref() { --m_iRefCount; - if(m_iRefCount == 0) + + // std::cout << "Unref, new count = " << m_iRefCount << std::endl; + + if(m_iRefCount == 0) { + // std::cout << "Deleting " << this << std::endl; delete this; + } }; /// diff --git a/Src/DasherCore/ConversionManagerFactory.cpp b/Src/DasherCore/ConversionManagerFactory.cpp index ae7891de..0ecc7d4d 100644 --- a/Src/DasherCore/ConversionManagerFactory.cpp +++ b/Src/DasherCore/ConversionManagerFactory.cpp @@ -46,7 +46,11 @@ CDasherNode *CConversionManagerFactory::GetRoot(CDasherNode *pParent, int iLower } else m_iCMCount++; - return pConversionManager->GetRoot(pParent, iLower, iUpper, pUserData); + + CDasherNode *pNewRoot = pConversionManager->GetRoot(pParent, iLower, iUpper, pUserData); + pConversionManager->Unref(); + + return pNewRoot; } // TODO: Japanese/Chinese are currently disabled in Win32 - see 'exclude from build' on individual files' property pages, plus preprocessor defines diff --git a/Src/DasherCore/DasherNode.cpp b/Src/DasherCore/DasherNode.cpp index ecf0f86c..c350ab67 100644 --- a/Src/DasherCore/DasherNode.cpp +++ b/Src/DasherCore/DasherNode.cpp @@ -40,11 +40,15 @@ static char THIS_FILE[] = __FILE__; // TODO: put this back to being inlined CDasherNode::~CDasherNode() { + // std::cout << "Deleting node: " << this << std::endl; // Release any storage that the node manager has allocated, // unreference ref counted stuff etc. + Delete_children(); + m_pNodeManager->ClearNode( this ); + m_pNodeManager->Unref(); - Delete_children(); + // std::cout << "done." << std::endl; delete m_pDisplayInfo; } @@ -139,11 +143,15 @@ void CDasherNode::Delete_children() { // if((GetDisplayInfo()->strDisplayText)[0] == 'e') // std::cout << "ed: " << this << " " << pParentUserData->iContext << " " << pParentUserData->iOffset << std::endl; +// std::cout << "Start: " << this << std::endl; + ChildMap::iterator i; for(i = Children().begin(); i != Children().end(); i++) { + // std::cout << "CNM: " << (*i)->m_pNodeManager << " (" << (*i)->m_pNodeManager->GetID() << ") " << (*i) << " " << (*i)->Parent() << std::endl; delete (*i); } Children().clear(); + // std::cout << "NM: " << m_pNodeManager << std::endl; SetFlag(NF_ALLCHILDREN, false); } diff --git a/Src/DasherCore/Makefile.am b/Src/DasherCore/Makefile.am index 4723154d..76d202ac 100644 --- a/Src/DasherCore/Makefile.am +++ b/Src/DasherCore/Makefile.am @@ -76,6 +76,7 @@ libdashercore_a_SOURCES = \ OneDimensionalFilter.cpp \ OneDimensionalFilter.h \ Parameters.h \ + SCENode.cpp \ SCENode.h \ SettingsStore.cpp \ SettingsStore.h \ diff --git a/Src/DasherCore/SCENode.cpp b/Src/DasherCore/SCENode.cpp new file mode 100644 index 00000000..5e561b2a --- /dev/null +++ b/Src/DasherCore/SCENode.cpp @@ -0,0 +1,33 @@ +#include "SCENode.h" + +SCENode::SCENode() { + m_pNext = 0; + m_pChild = 0; + + m_iRefCount = 1; +} + +SCENode::~SCENode() { + if(m_pNext) + m_pNext->Unref(); + + if(m_pChild) + m_pChild->Unref(); +} + +void SCENode::SetNext(SCENode *pNext) { + if(m_pNext) + m_pNext->Unref(); + + m_pNext = pNext; + m_pNext->Ref(); +}; + +void SCENode::SetChild(SCENode *pChild) { + if(m_pChild) + m_pChild->Unref(); + + m_pChild = pChild; + m_pChild->Ref(); +}; + diff --git a/Src/DasherCore/SCENode.h b/Src/DasherCore/SCENode.h index 03bb3899..e62d4e58 100644 --- a/Src/DasherCore/SCENode.h +++ b/Src/DasherCore/SCENode.h @@ -1,31 +1,60 @@ - #ifndef __SCENODE_H__ #define __SCENODE_H__ /*Common Node Definition for Chinese Pinyin (possibly also Japanese) Conversion Library and Dasher ConversionManager*/ -typedef struct _SCENode SCENode; /// \ingroup Model /// \{ -struct _SCENode { +class SCENode { + public: + SCENode(); + + ~SCENode(); + + void Ref() { + ++m_iRefCount; + }; + + void Unref() { + --m_iRefCount; + + if(m_iRefCount == 0) { + delete this; + } + }; + + SCENode *GetNext() { + return m_pNext; + }; + + void SetNext(SCENode *pNext); + + SCENode *GetChild() { + return m_pChild; + }; + + void SetChild(SCENode *pChild); char *pszConversion; - SCENode *pNext; - SCENode *pChild; int IsHeadAndCandNum; int CandIndex; int Symbol; - + int IsComplete; int AcCharCount; /*accumulative character count*/ - + int NodeSize; - + unsigned int HZFreq; float HZProb; + private: + int m_iRefCount; + + SCENode *m_pNext; + SCENode *m_pChild; }; /// \} diff --git a/Src/Gtk2/Canvas.cpp b/Src/Gtk2/Canvas.cpp index 6c442ed7..3ef7e0e9 100644 --- a/Src/Gtk2/Canvas.cpp +++ b/Src/Gtk2/Canvas.cpp @@ -236,7 +236,7 @@ void CCanvas::DrawRectangle(int x1, int y1, int x2, int y2, int Color, int iOutl #if WITH_CAIRO cairo_set_line_width(cr, iThickness); - cairo_rectangle(cr, iLeft + 0.5, iTop + 0.5, iWidth - 1, iHeight - 1); + cairo_rectangle(cr, iLeft + 0.5, iTop + 0.5, iWidth, iHeight); cairo_stroke(cr); #else gdk_gc_set_line_attributes(graphics_context, iThickness, GDK_LINE_SOLID, GDK_CAP_ROUND, GDK_JOIN_ROUND ); -- 2.11.4.GIT