[Scidvspc-users] [PATCH] fix memory leak in pgn parser (wrt. character detector)

[Ali P. <al...@ex...>]

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 <al...@ex...>
---
 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