Many thanks for that Ali. :)
Steven
On Tue, Oct 4, 2016 at 6:18 AM, Ali Polatel <a...@exherbo.org> wrote:
> Hello everyone,
>
> I have spotted a memory leak in the pgn parser. This makes Scid pretty much
> unusuable for me after working on the tree window for a while.
>
> Below is the valgrind output of the incident:
>
> 138,880 bytes in 248 blocks are definitely lost in loss record 774 of 818
> at 0x4C2A0FC: operator new(unsigned long) (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> by 0x44E57D: PgnParser::Reset(char const*) (in /usr/bin/tkscid)
> by 0x45358E: StoredLine::Init() (in /usr/bin/tkscid)
> by 0x453748: StoredLine::StoredLine(Position*) (in /usr/bin/tkscid)
> by 0x42D23C: sc_tree_search(void*, Tcl_Interp*, int, char const**) (in
> /usr/bin/tkscid)
> by 0x42EBEF: sc_tree(void*, Tcl_Interp*, int, char const**) (in
> /usr/bin/tkscid)
> by 0x50889B5: TclInvokeStringCommand (in /usr/lib/libtcl8.6.so)
> by 0x508D5F6: TclNRRunCallbacks (in /usr/lib/libtcl8.6.so)
> by 0x508F701: TclEvalEx (in /usr/lib/libtcl8.6.so)
> by 0x508FCB2: Tcl_EvalEx (in /usr/lib/libtcl8.6.so)
> by 0x543DFAD: Tk_BindEvent (in /usr/lib/libtk8.6.so)
> by 0x5441F5C: TkBindEventProc (in /usr/lib/libtk8.6.so)
>
> The problem is PgnParser::Reset() is be called from multiple
> different functions and during some cases CharConverter is
> already allocated. Overwriting the variable to NULL causes
> the previously allocated memory to leak.
>
> To reproduce open a giant scid database in the tree and browse the games
> for a while. In about half an hour scid running on a 4G RAM box eats all
> the memory and hits OOM.
>
> The change moves the initialisation of CharConverter and CharDetector to
> PgnParser::Init(). We do not need to set them to NULL in PgnParser::Reset()
> due to the fact that all callers also call CreateCharsetDetector() which
> takes
> care of the initialisation.
>
> I have been running Scid with this patch all day today and it seems to
> cause no regressions and the memory usage is OK.
>
> Signed-off-by: Ali Polatel <a...@exherbo.org>
> ---
> src/pgnparse.cpp | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/pgnparse.cpp b/src/pgnparse.cpp
> index bdfc4f9..a49e894 100644
> --- a/src/pgnparse.cpp
> +++ b/src/pgnparse.cpp
> @@ -37,6 +37,8 @@ void
> PgnParser::Init ()
> {
> ErrorBuffer = new DString;
> + CharConverter = NULL;
> + CharDetector = NULL;
> Reset();
> }
>
> @@ -56,8 +58,6 @@ PgnParser::Reset()
> ResultWarnings = true;
> NewlinesToSpaces = true;
> NumIgnoredTags = 0;
> - CharConverter = NULL;
> - CharDetector = NULL;
> }
>
> void
> --
> 2.10.0
>
> ------------------------------------------------------------
> ------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Scidvspc-users mailing list
> Scidvspc-users@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/scidvspc-users
>
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Scidvspc-users mailing list
Scidvspc-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/scidvspc-users
