Page MenuHomePhabricator

memcheck errors in mitkNavigationToolReaderAndWriterTest
Closed, ResolvedPublic

Description

valgrind run

http://mbits/cdash/viewDynamicAnalysis.php?buildid=22053

shows 29 errors in mitkNavigationToolReaderAndWriterTest. These should be examined.

If the errors can be removed, a fix should be applied. If they are false positives generated by valgrind, they should be suppressed (in this case please talk to Daniel M.).

Event Timeline

[SVN revision 21333]
FIX (#3343): use vtkSmartPointers where appropriate to reduce potential for memory errors

[SVN revision 21338]
FIX (#3343): removed unused variable; unnecessary Delete

Adding test author to cc list. Alfred, please comment.

re-assigning to class author.

I tried to fix/undestand this bug today but failed. I'll not work next week because of my beginning study lessons, so I reassigned the bug.

details:

The bug is only reproduceable in a linux environment which was the main handicap for me because I'm normally working with windows. (the software valgrind is needed to reproduce this bug)

According to error output on cdash I assume that the error is not caused by the class mitkNavigationToolReaderAndWriterTest or mitkSTLFileReader but by a vtk class.

Thiago, could you please help Alfred with this?

There are no memory leaks. Valgrind is only pointing that there are operations with uninitialized variables.

In the first case for example, in the vtkInformation destructor, the this->Internal member variable is destroyed, but since Valgrind couldn't find its initialization, a warning is issued in order to point a unsafe destruction. The variable was actually initialized in the constructor, but inside the method. The correct way to do it would be by using constructor initializer.

In mitk::NavigationTool::SetTrackingDeviceType(mitk::TrackingDeviceType) the same problem happens, because m_TrackingDeviceType was not initialized in the constructor.

Since the direct use of constructors with appropriate parameters are not used in MITK, VTK and ITK because of the smart pointers, and because of the fact that that all internal variables are initialized through setters and getters, the programmers usually forget to initialize them in the constructor with constructor initializers.

I guess there is no much we can do about that.

As discussed today:

  • if valgrind complaints from VTK files are false positives (i.e. you are sure that the code is right and nothing could go wrong), we can write suppressions (I'll assist you next Wednesday)
  • if valgrind complaints from VTK files are serious, please reference a VTK bug here (find or file it, if possible with a patch)
  • if valgrind complaints come from MITK (you mention one such case in your last comment) then we should be able to fix the issue. Fixing such issues was the main intention of this bug report

Some errors were suppressed. The errors that are still there can be fixed by the author. Valgrind gives very specific information about the location of the errors and what it is all about.

I checked all remaining errors but couldn't find any possible reasons for them in the mitk code. So I assume this are errors of Poco and vtk which we can't fix. After consulting Daniel I would propose to supress this errors like the others which are caused by external libraries.

Resetting all bugs without active assignee flag to "CONFIRMED". Change status to IN_PROGRESS if you are working on it.

resetting bug to confirmed since they are not currently in progress

when running the test with valgrind --leak-check=full there only several but always the same warnings due to an uninitialised value with a call stack starting mitkSceneIO.cpp:442 :


17763== 68 errors in context 10 of 25:

17763== Conditional jump or move depends on uninitialised value(s)

17763== at 0xA81AEE0: longest_match (deflate.c:1121)

17763== by 0xA81C1EB: deflate_slow (deflate.c:1595)

17763== by 0xA81A379: deflate (deflate.c:790)

17763== by 0xA771352: Poco::DeflatingStreamBuf::close() (DeflatingStream.cpp:112)

17763== by 0xA772EFB: Poco::DeflatingOutputStream::close() (DeflatingStream.cpp:271)

17763== by 0x10665D5D: Poco::Zip::ZipStreamBuf::close() (ZipStream.cpp:219)

17763== by 0x10667277: Poco::Zip::ZipOutputStream::close() (ZipStream.cpp:314)

17763== by 0x106467DC: Poco::Zip::Compress::addEntry(std::istream&, Poco::DateTime const&, Poco::Path const&, Poco::Zip::ZipCommon::CompressionMethod, Poco::Zip::ZipCommon::CompressionLevel) (Compress.cpp:83)

17763== by 0x10647A56: Poco::Zip::Compress::addFile(std::istream&, Poco::DateTime const&, Poco::Path const&, Poco::Zip::ZipCommon::CompressionMethod, Poco::Zip::ZipCommon::CompressionLevel) (Compress.cpp:170)

17763== by 0x10647E60: Poco::Zip::Compress::addFile(Poco::Path const&, Poco::Path const&, Poco::Zip::ZipCommon::CompressionMethod, Poco::Zip::ZipCommon::CompressionLevel) (Compress.cpp:185)

17763== by 0x1064925E: Poco::Zip::Compress::addRecursive(Poco::Path const&, Poco::Zip::ZipCommon::CompressionLevel, bool, Poco::Path const&) (Compress.cpp:271)

17763== by 0x8C4B651: mitk::SceneIO::SaveScene(itk::SmartPointer<itk::VectorContainer<unsigned int, itk::SmartPointer<mitk::DataNode> > const>, mitk::DataStorage const*, std::string const&) (mitkSceneIO.cpp:442)


after investigation of mitkSceneIO.cpp we can say that there is no unitialised value in mitk related code but in the Poco implementation.

does the test still fail somehow. the reported uninitialised value warnings may only show up as warnings when poco is compiled. as far as i can remember those warnings are suppressed anyway?
any suggestions? otherwise, i will close this bug

test runs fine on the dartclients:
http://cdash.mitk.org/testSummary.php?project=1&name=mitkNavigationToolReaderAndWriterTest&date=2012-02-29

last valgrind checks only shows up warnings due to uninitialised poco variables. closing bug