Re: [sleuthkit-developers] memory leak and a patch

[Stefan P. <ste...@gm...>]

Patch attached

On Tue, May 06, 2014 at 04:01:42AM +0300, Stefan Petrea wrote:
> Sending a new patch. Forgot to add a line in the previous one.
> I've opened a github pull request for this here https://github.com/sleuthkit/sleuthkit/pull/329
> 
> On Tue, May 06, 2014 at 03:41:09AM +0300, Stefan Petrea wrote:
> > Hi,
> > 
> > I'm using Sleuthkit and encountered a memory leak in TskFsDir::getFile. On a 20GB disk image,
> > with ~10k files this leads to unallocated memory of ~2.5GB according to Valgrind.
> >     
> > I'm writing to the mailing list to provide details on the leak and a patch for it.
> > 
> > I wrote a small testcase that reproduces the leak in isolated.cpp
> > 
> > The output of Valgrind before the patch was applied( commit a9e2aa0e39cd8eeffd4cccf951b7b91a40f5e8c0 ), 
> > can be found in valgrind-output-before-patch.txt and the output of Valgrind after the patch 
> > was applied can be found in valgrind-output-after-patch.txt
> > 
> > The leaks reported by Valgrind all originate in TskFsDir::getFile. The unfreed objects
> > were all of type TskFsFile.
> > 
> > The script find-leak.py automates GDB 7.6 and is designed to automatically debug isolated.cpp and
> > at the same time collect information about the callgraph and pointers that cause the leak
> > (Note: GDB 7.7 has a different Python API).
> > 
> > find-leak.py keeps track of all the pointers allocated through tsk_fs_file_alloc and all
> > the pointers deallocated through tsk_fs_file_close which can be seen in the callgraph
> > gdb-all-paths.png
> > 
> > The aim of this script (find-leak.py ) is to track the unfreed objects and find the call paths that lead
> > to their allocation. This can be seen in the callgraph gdb-paths-that-caused-leak.png 
> > which confirms Valgrind's report. The difference between this callgraph and the previous one is
> > that this one only tracks the objects that have not been deallocated.
> > 
> > The patch that is attached to this e-mail contains a fix for this. It works by making TskFsDir a friend
> > class of TskFsFile in order for TskFsDir::getFile to be able to set the TskFsFile::m_opened attribute
> > to true, since if that attribute is not set to true, TskFsFile::~TskFsFile will not call
> > tsk_fs_file_close which will cause the leak to occur.
> > 
> > I will also make a pull-request on github for this.
> > 
> > I look forward to your opinion on this patch. 
> > 
> > Best regards,
> > Stefan
> > 
> > P.S.
> > 
> > As a sidenote, Valgrind also reported a leak found in tsk/base/tsk_error.c , originating from a call to
> > tsk_error_get_info. I've noticed that there is a destructor handler free_error_info which deallocates
> > the memory allocated by tsk_error_get_info, but that destructor is only called
> > upon pthread_exit. The valgrind leak report for this can be found in valgrind-leak-tsk-error.txt
> > To fix this, isolated.cpp also calls pthread_exit which solves that problem.