Memory leaks (windows) in dcmtk-3.6.1_20110225.tar.gz
Moderator: Moderator Team
Memory leaks (windows) in dcmtk-3.6.1_20110225.tar.gz
Dear forum readers,
I have tried the newest snapshot (dcmtk-3.6.1_20110225.tar.gz) and I think I have found that in the windows versions (I am using Visual Studio 2008 32bit) there are some memory leaks when linking my application to dcmdata.lib, ofstd.lib, oflog.lib.
I have not had the same problem with version 3.6.0.
I think that the memory not released when the application is closed is allocated in the oflog/libsrc/globinit.cc line 90 during the call to the function log4cplus::initializeLog4cplus();
Looking to the modifications between 3.6.0 and 3.6.1_20110225 I have found that you removed the virtual keyword from the destructor of OFListLink, OFListBase, OFListLinkBase in ofstd/include/dcmtk/ofstd/oflist.h
If I restore the virtual keyword in the destructors of the list classes I cannot see the memory leaks anymore. I hope this can help.
Best regards,
Valerio
I have tried the newest snapshot (dcmtk-3.6.1_20110225.tar.gz) and I think I have found that in the windows versions (I am using Visual Studio 2008 32bit) there are some memory leaks when linking my application to dcmdata.lib, ofstd.lib, oflog.lib.
I have not had the same problem with version 3.6.0.
I think that the memory not released when the application is closed is allocated in the oflog/libsrc/globinit.cc line 90 during the call to the function log4cplus::initializeLog4cplus();
Looking to the modifications between 3.6.0 and 3.6.1_20110225 I have found that you removed the virtual keyword from the destructor of OFListLink, OFListBase, OFListLinkBase in ofstd/include/dcmtk/ofstd/oflist.h
If I restore the virtual keyword in the destructors of the list classes I cannot see the memory leaks anymore. I hope this can help.
Best regards,
Valerio
-
- DCMTK Developer
- Posts: 120
- Joined: Thu, 2009-11-26, 08:15
Re: Memory leaks (windows) in dcmtk-3.6.1_20110225.tar.gz
Hi,
On Linux, I ran both dcmdump and storescp under valgrind. In both cases it didn't detect any memory leak in oflog.
Cheers,
Uli
Why do you think that this is where the memleak occurs? How do you check for memory leaks? Since initializeLog4cplus() calls various functions, can you test which one of these causes the problem?vsalomoni wrote:I think that the memory not released when the application is closed is allocated in the oflog/libsrc/globinit.cc line 90 during the call to the function log4cplus::initializeLog4cplus();
On Linux, I ran both dcmdump and storescp under valgrind. In both cases it didn't detect any memory leak in oflog.
This was changed because OFList is supposed to be a portable implementation of std::list (and when HAVE_STL is defined, OFList turns into a simple typedef for std::list). Since std::list isn't virtual, OFList shouldn't be virtual either.vsalomoni wrote:Looking to the modifications between 3.6.0 and 3.6.1_20110225 I have found that you removed the virtual keyword from the destructor of OFListLink, OFListBase, OFListLinkBase in ofstd/include/dcmtk/ofstd/oflist.h
Cheers,
Uli
Re: Memory leaks (windows) in dcmtk-3.6.1_20110225.tar.gz
Hello Uli,
In the mean time I have made many tests on the Offis source code and in my opinion the virtual keyword is needed at least in the base classes you are using (OFListBase, OFListLinkBase).
What happens is that if you don't use the virtual destructor in the OFListLinkBase class, when you delete the OFListLink<T> objects created in the OFList<T> class you just call the destructor of the OFListLinkBase class. Both destructors are empty, thus avoiding the call to the OFListLink class destructor would not be a big problem, but the real problem is that in class OFListLink the destructor of the class member "T info;" is not called (and when you make list of complex data you need to call it).
Here is a code sample of what happens (simplyfing the situation as much as possible):
In this sample the destructor of T and DemoListLink classes are never called and the string allocated in the T class is never freed.
At the end I would suggest to restore the virtual keywords at least for the two base classes (OFListBase, OFListLinkBase).
Regards,
Valerio
I've checked memory leaks with the Visual Studio 2009 using MFC and Debug flags (which calls the _CrtDumpMemoryLeaks function before closing application).Uli Schlachter wrote: Hi,Why do you think that this is where the memleak occurs? How do you check for memory leaks? Since initializeLog4cplus() calls various functions, can you test which one of these causes the problem?vsalomoni wrote:I think that the memory not released when the application is closed is allocated in the oflog/libsrc/globinit.cc line 90 during the call to the function log4cplus::initializeLog4cplus();
On Linux, I ran both dcmdump and storescp under valgrind. In both cases it didn't detect any memory leak in oflog.This was changed because OFList is supposed to be a portable implementation of std::list (and when HAVE_STL is defined, OFList turns into a simple typedef for std::list). Since std::list isn't virtual, OFList shouldn't be virtual either.vsalomoni wrote:Looking to the modifications between 3.6.0 and 3.6.1_20110225 I have found that you removed the virtual keyword from the destructor of OFListLink, OFListBase, OFListLinkBase in ofstd/include/dcmtk/ofstd/oflist.h
Cheers,
Uli
In the mean time I have made many tests on the Offis source code and in my opinion the virtual keyword is needed at least in the base classes you are using (OFListBase, OFListLinkBase).
What happens is that if you don't use the virtual destructor in the OFListLinkBase class, when you delete the OFListLink<T> objects created in the OFList<T> class you just call the destructor of the OFListLinkBase class. Both destructors are empty, thus avoiding the call to the OFListLink class destructor would not be a big problem, but the real problem is that in class OFListLink the destructor of the class member "T info;" is not called (and when you make list of complex data you need to call it).
Here is a code sample of what happens (simplyfing the situation as much as possible):
Code: Select all
struct T
{
T::T()
{
printf("Constructor T\n");
m_string = new char[256];
}
T::~T()
{
printf("Destructor T\n");
delete [] m_string;
}
char *m_string;
};
struct DemoListLinkBase
{
DemoListLinkBase::DemoListLinkBase()
{
printf("Constructor DemoListLinkBase\n");
}
DemoListLinkBase::~DemoListLinkBase()
{
printf("Destructor DemoListLinkBase\n");
}
};
struct DemoListLink : public DemoListLinkBase
{
T info;
DemoListLink::DemoListLink()
{
printf("Constructor DemoListLink\n");
}
DemoListLink::~DemoListLink()
{
printf("Destructor DemoListLink\n");
}
};
int main(int argc, char *argv[])
{
DemoListLinkBase *BasePointerToDerivedClass = new DemoListLink;
delete BasePointerToDerivedClass;
return 0;
}
The output is the following:
Constructor DemoListLinkBase
Constructor T
Constructor DemoListLink
Destructor DemoListLinkBase
At the end I would suggest to restore the virtual keywords at least for the two base classes (OFListBase, OFListLinkBase).
Regards,
Valerio
-
- DCMTK Developer
- Posts: 120
- Joined: Thu, 2009-11-26, 08:15
Sorry, I was compiling DCMTK with HAVE_STL which means that OFList was replaced with std::list. This is why I didn't see any memory leaks.
Of course you are right. I have made the destructors of OFListLinkBase and OFListBase virtual again which fixes this problem. This change should appear in git soon.
Uli
Edit:
Here's the link:
http://git.dcmtk.org/web?p=dcmtk.git;a= ... f24d2d6ad2
Of course you are right. I have made the destructors of OFListLinkBase and OFListBase virtual again which fixes this problem. This change should appear in git soon.
Uli
Edit:
Here's the link:
http://git.dcmtk.org/web?p=dcmtk.git;a= ... f24d2d6ad2
I recently downloaded dcmtk-3.6.1_200110707.tar.gz, and a tool I use to detect memory leaks has gone wild with traces of the like:
9122: f:\vs70builds\6030\vc\mfcatl\ship\atlmfc\src\mfc\afxmem.cpp(343) +16 bytes (operator new)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\ofstd\libsrc\oflist.cc(44) +7 bytes (OFListBase::OFListBase)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\ofstd\include\dcmtk\ofstd\oflist.h(329) +15 bytes (OFList<DcmDictEntry *>::OFList<DcmDictEntry *>)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\include\dcmtk\dcmdata\dchashdi.h(53) +15 bytes (DcmDictEntryList::DcmDictEntryList)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dchashdi.cc(471) +34 bytes (DcmHashDict::put)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdict.cc(686) +0 bytes (DcmDataDictionary::addEntry)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdictzz.cc(24991) +0 bytes (DcmDataDictionary::loadBuiltinDictionary)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdict.cc(424) +0 bytes (DcmDataDictionary::reloadDictionaries)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdict.cc(125) +0 bytes (DcmDataDictionary::DcmDataDictionary)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdict.cc(823) +38 bytes (GlobalDcmDataDictionary::createDataDict)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdict.cc(842) +0 bytes (GlobalDcmDataDictionary::rdlock)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dctag.cc(128) +10 bytes (DcmTag::lookupVRinDictionary)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dctag.cc(66) +0 bytes (DcmTag::DcmTag)
9122: d:\rootreferences\xvisionapi\xvision_core\cxvtextpresenter.cpp(699) +21 bytes (CxvTextPresenter::getConvertedString)
9122: d:\rootreferences\xvisionapi\xvision_core\cxvtextpresenter.cpp(162) +35 bytes (CxvTextPresenter::createTextPresenterItemsFrom)
The line in my code just declares a tag so:
DcmTag tag(iGroup, iElement);
That tool has never failed in the past, so something smells fishy. Previous versions of dcmtk did not get reported by my tool. What the tool does is basically override new and delete and reports any mismatches.
Can you guys check into this?
Thanks
9122: f:\vs70builds\6030\vc\mfcatl\ship\atlmfc\src\mfc\afxmem.cpp(343) +16 bytes (operator new)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\ofstd\libsrc\oflist.cc(44) +7 bytes (OFListBase::OFListBase)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\ofstd\include\dcmtk\ofstd\oflist.h(329) +15 bytes (OFList<DcmDictEntry *>::OFList<DcmDictEntry *>)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\include\dcmtk\dcmdata\dchashdi.h(53) +15 bytes (DcmDictEntryList::DcmDictEntryList)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dchashdi.cc(471) +34 bytes (DcmHashDict::put)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdict.cc(686) +0 bytes (DcmDataDictionary::addEntry)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdictzz.cc(24991) +0 bytes (DcmDataDictionary::loadBuiltinDictionary)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdict.cc(424) +0 bytes (DcmDataDictionary::reloadDictionaries)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdict.cc(125) +0 bytes (DcmDataDictionary::DcmDataDictionary)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdict.cc(823) +38 bytes (GlobalDcmDataDictionary::createDataDict)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dcdict.cc(842) +0 bytes (GlobalDcmDataDictionary::rdlock)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dctag.cc(128) +10 bytes (DcmTag::lookupVRinDictionary)
9122: e:\rootreferences\dcmtk-3.6.1_20110707\dcmdata\libsrc\dctag.cc(66) +0 bytes (DcmTag::DcmTag)
9122: d:\rootreferences\xvisionapi\xvision_core\cxvtextpresenter.cpp(699) +21 bytes (CxvTextPresenter::getConvertedString)
9122: d:\rootreferences\xvisionapi\xvision_core\cxvtextpresenter.cpp(162) +35 bytes (CxvTextPresenter::createTextPresenterItemsFrom)
The line in my code just declares a tag so:
DcmTag tag(iGroup, iElement);
That tool has never failed in the past, so something smells fishy. Previous versions of dcmtk did not get reported by my tool. What the tool does is basically override new and delete and reports any mismatches.
Can you guys check into this?
Thanks
-
- DCMTK Developer
- Posts: 2503
- Joined: Tue, 2011-05-03, 14:38
- Location: Oldenburg, Germany
- Contact:
Some of the DCMTK command line tools contain the following code at the end of the main function (before return):
Maybe, this is also useful to convince your checking tool that there is no memory leak.
The problem is that your code initializes the global data dictionary, which is usually never cleared (unless something like the above is used in the code before the program exists).
Code: Select all
#ifdef DEBUG
dcmDataDict.clear(); /* useful for debugging with dmalloc */
#endif
The problem is that your code initializes the global data dictionary, which is usually never cleared (unless something like the above is used in the code before the program exists).
-
- DCMTK Developer
- Posts: 2503
- Joined: Tue, 2011-05-03, 14:38
- Location: Oldenburg, Germany
- Contact:
You are right. I guess this is the corresponding commit. It has been introduced in order to solve other issues that were present with the "old approach".
Of course, the global data dictionary is cleared in the destructor of the class, but that's too late for most memory leak checking tools
Of course, the global data dictionary is cleared in the destructor of the class, but that's too late for most memory leak checking tools
-
- DCMTK Developer
- Posts: 2049
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
-
- DCMTK Developer
- Posts: 120
- Joined: Thu, 2009-11-26, 08:15
Should be fixed now. I tested with valgrind. dcmqrscp was run with --allow-shutdown --single-process and the only SCU used was termscu.
Valgrind now reports:
Uli
Edit:
http://git.dcmtk.org/web?p=dcmtk.git;a= ... 8bad6d88fc
Valgrind now reports:
A link to the git commit will be given as soon as this appears in git.All heap blocks were freed -- no leaks are possible
Uli
Edit:
http://git.dcmtk.org/web?p=dcmtk.git;a= ... 8bad6d88fc
Who is online
Users browsing this forum: Semrush [Bot] and 1 guest