Memory Leaks

[mark]

I found some memory leaks with valgrind. One leak is definitely in the Logger code: a new Logger object is created and returned and never destroyed but I don't understand how the Logger is designed so I don't fix this by myself.

Here is some output from 'valgrind --tool=memcheck --leak-check=yes ./gemrb'

==30905== 16 bytes in 1 blocks are definitely lost in loss record 20 of 502
==30905==    at 0x4C2C280: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30905==    by 0x508F1F0: GemRB::createStdioLogger() (Stdio.cpp:121)
==30905==    by 0x508F2E2: GemRB::InitializeLogging() (Logging.cpp:41)
==30905==    by 0x109174: main (GemRB.cpp:82)
 
==30905== 280 bytes in 1 blocks are possibly lost in loss record 327 of 502
==30905==    at 0x4C2C840: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30905==    by 0x8BEA438: _PyObject_GC_Malloc (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BEA52C: _PyObject_GC_New (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8B5DC23: PyDict_New (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8B9D5F1: _PyWarnings_Init (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BD8DD7: Py_InitializeEx (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x88BE33D: GemRB::GUIScript::Init() (GUIScript.cpp:11033)
==30905==    by 0x4F59579: GemRB::Interface::Init(GemRB::InterfaceConfig*) (Interface.cpp:1776)
==30905==    by 0x1091CE: main (GemRB.cpp:86)
 
==30905== 456 bytes in 1 blocks are possibly lost in loss record 346 of 502
==30905==    at 0x4C2C840: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30905==    by 0x8BEA438: _PyObject_GC_Malloc (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BEA56D: _PyObject_GC_NewVar (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8B4C8CE: PyFrame_New (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BBBFBC: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BBD3E1: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8B4D055:  (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8B283AD: PyObject_Call (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BB6B76: PyEval_CallObjectWithKeywords (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x88BEF43: GemRB::GUIScript::RunFunction(char const*, char const*, _object*, bool) (GUIScript.cpp:11255)
==30905==    by 0x88BF02A: GemRB::GUIScript::RunFunction(char const*, char const*, bool, int) (GUIScript.cpp:11273)
==30905==    by 0x4F53052: GemRB::Interface::HandleFlags() (Interface.cpp:689)
 
==30905== 1,408 bytes in 3 blocks are possibly lost in loss record 423 of 502
==30905==    at 0x4C2C840: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30905==    by 0x8BEA438: _PyObject_GC_Malloc (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BEA56D: _PyObject_GC_NewVar (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8B4C8CE: PyFrame_New (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BBCC55: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BBB6EB: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BBD3E1: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8B4D055:  (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8B283AD: PyObject_Call (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x8BB6B76: PyEval_CallObjectWithKeywords (in /usr/lib64/libpython2.7.so.1.0)
==30905==    by 0x88BEF43: GemRB::GUIScript::RunFunction(char const*, char const*, _object*, bool) (GUIScript.cpp:11255)
==30905==    by 0x88BF02A: GemRB::GUIScript::RunFunction(char const*, char const*, bool, int) (GUIScript.cpp:11273)
 
==30905== 4,192 (24 direct, 4,168 indirect) bytes in 1 blocks are definitely lost in loss record 477 of 502
==30905==    at 0x4C2C280: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30905==    by 0x508E5B6: GemRB::createFileLogger(GemRB::DataStream*) (File.cpp:51)
==30905==    by 0x508E696: GemRB::addLogger() (File.cpp:60)
==30905==    by 0x4FA5ACC: GemRB::PluginMgr::RunInitializers() const (PluginMgr.cpp:87)
==30905==    by 0x4F58A26: GemRB::Interface::Init(GemRB::InterfaceConfig*) (Interface.cpp:1658)
==30905==    by 0x1091CE: main (GemRB.cpp:86)

 

Could you enable the github issue page for gemrb?

[SyntaxError]

when doing a Valgrind check on GemRB you should use pass the suppression files found under the "testing" directory.

 

Not saying it shouldn't be fixed, but if that leak is valid, it's a one time object that persists throughout the program so...

 

Could you enable the github issue page for gemrb?

I wish. asked about that before, but others seem to prefer the Sourceforge bugzilla.

[lynx]

There's no sense in further fragmenting the bug doc space.

 

The suppression file deals with external bugs (mostly python as above) and you can find it at testing/python.supp . The logger thing looks legit.

[SyntaxError]

There's no sense in further fragmenting the bug doc space.

True, but is there not a trivial means to export the issues from SF and import them to github? using 2 different systems is fragmentation and requires 2 different accounts.

[lynx]

github can't cater to all our needs. I'm always advocating for the wiki tracker anyway.

[Avenger]

One time object leaks are not real leaks. I'm amazed you didn't find more, the last valgrind run (i know about) was years ago.

[lynx]

Uh, we run it through valgrind several times per year, just not systematically. My laptop would melt if I let it run for more than a few minutes.

[AnonymousHero]

(I realize that it's been a while since this was posted...)

 

Would it make sense to systematically start using unique_ptr<> from C++11 instead of raw pointers? I mean as a coding style thing?

 

Anecdotally, simply doing this has increased leak-resistance enormously in a pet project of mine. Not quite a project on the scale of GemRB, but usually these improvements only get better with project size.

 

Unfortunately there's no make_unique_ptr<> in C++11 (for exception safety), but it's pretty simple to write and include in the project (Herb Sutter has an implementation posted somewhere, I think).

[SyntaxError]

last time I asked about using C++11 stuff I was told "no". If that has changed i may be a bit irked since this massive change-set im working one would have been a bit easier with C++11

[lynx]

our exotic platforms would cry, so no, not yet.