[PATCH] Memory Leaks

[Cyrus Cousins]

There are many memory leaks and memory errors in 0AD (though most of the leaks are trivial one time events caused by startup and shutdown). Valgrind has reported where memory was allocated for all leaked (definitely lost memory). I have attached memory leak summary reported by valgrind after getting to the title screen and exiting normally.

[Cyrus Cousins]

Valgrind leak summary .

[Cyrus Cousins]

Memory Errors

[Cyrus Cousins]

Patch for SoundManager.cpp

[Cyrus Cousins]

This file contains a full output of valgrind, including: All leaked memory, and where it was allocated, and All memory errors, and where corresponding memory was allocated.

[ben]

In 12987:

Fixes memory leak in sound manager, refs #1736

[historic_bruno]

Thanks for the patch, I made minor changes and committed it.

Looking at a few of the other sound related leaks, it seems we never call ​ov_clear() which is required since we use ​ov_open_callbacks(). I guess that's where leaks like this come from:

==2244== 80,379 (56 direct, 80,323 indirect) bytes in 1 blocks are definitely lost in loss record 239 of 240
==2244==    at 0x4C2B7B2: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2244==    by 0x6A64AF8: ??? (in /usr/lib/x86_64-linux-gnu/libvorbisfile.so.3.3.4)
==2244==    by 0x6A65EE6: ??? (in /usr/lib/x86_64-linux-gnu/libvorbisfile.so.3.3.4)
==2244==    by 0x6A66067: ov_open_callbacks (in /usr/lib/x86_64-linux-gnu/libvorbisfile.so.3.3.4)
==2244==    by 0x5F7FA1: OpenOggNonstream(boost::shared_ptr<IVFS> const&, Path const&, boost::shared_ptr<OggStream>&) (ogg.cpp:229)
==2244==    by 0x5A55F3: COggData::InitOggFile(Path const&) (OggData.cpp:49)
==2244==    by 0x5A49C4: CSoundData::SoundDataFromFile(Path const&) (SoundData.cpp:104)
==2244==    by 0x5A33FA: CSoundManager::LoadItem(Path const&) (SoundManager.cpp:368)
==2244==    by 0x5AD9AD: CNativeFunction<JMusicSound, false, bool, &(JMusicSound::Loop(JSContext*, unsigned int, unsigned long*))>::JSFunction(JSContext*, unsigned int, unsigned long*) (MusicSound.cpp:53)
==2244==    by 0x57E61F8: js::Interpret(JSContext*, JSStackFrame*, unsigned int, JSInterpMode) (in /home/ccousins/0ADdev/0ad/binaries/system/libmozjs185-ps-release.so.1.0)
==2244==    by 0x57EF904: js::RunScript(JSContext*, JSScript*, JSStackFrame*) (in /home/ccousins/0ADdev/0ad/binaries/system/libmozjs185-ps-release.so.1.0)
==2244==    by 0x57EFDA1: js::Invoke(JSContext*, js::CallArgs const&, unsigned int) (in /home/ccousins/0ADdev/0ad/binaries/system/libmozjs185-ps-release.so.1.0)

[Cyrus Cousins]

A few list related leaks

[Cyrus Cousins]

I have patched a few more small memory leaks related to a few lists in the SoundManager file, however these are unfortunately trivial compared to the actual sound data leaked here. As historic_bruno mentions, ov_clear() is not called, although ov_open_callbacks() is called in ogg.cpp. I believe a large leak (that presumably builds up over time) could be avoided by calling this function, but I can't figure out where the sound destruction code is called.

[stwf]

Im in the process of rewriting the soundmanager code extensively, I will address all memory leak issues after the changes are complete.

[historic_bruno]

Looks like stwf fixed most of the sound-related leaks with r13209. A few smaller ones remain:

==2658== 24 bytes in 1 blocks are definitely lost in loss record 46 of 270
==2658==    at 0x4C28B35: operator new(unsigned long) (vg_replace_malloc.c:261)
==2658==    by 0x5BEB25: CSoundManager::CSoundManager() (SoundManager.cpp:47)
==2658==    by 0x5BED0C: CSoundManager::CreateSoundManager() (SoundManager.cpp:231)
==2658==    by 0x5A7289: Init(CmdLineArgs const&, int) (GameSetup.cpp:886)
==2658==    by 0x41D9BD: RunGameOrAtlas(int, char const**) (main.cpp:504)
==2658==    by 0x4136F6: main (main.cpp:551)

==2658== 32 (24 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 66 of 270
==2658==    at 0x4C28B35: operator new(unsigned long) (vg_replace_malloc.c:261)
==2658==    by 0x5C0261: CSoundManagerWorker::RunThread(void*) (SoundManager.cpp:155)
==2658==    by 0x80C4EFB: start_thread (pthread_create.c:304)
==2658==    by 0x83BEF8C: clone (clone.S:112)

==2658== 124 bytes in 1 blocks are definitely lost in loss record 191 of 270
==2658==    at 0x4C28F9F: malloc (vg_replace_malloc.c:236)
==2658==    by 0xA4E9564: ??? (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
==2658==    by 0xA4E962A: ??? (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
==2658==    by 0xA4E91CA: xcb_connect_to_display_with_auth_info (in /usr/lib/x86_64-linux-gnu/libxcb.so.1.1.0)
==2658==    by 0xD3910ED: pa_client_conf_from_x11 (in /usr/lib/x86_64-linux-gnu/libpulsecommon-1.0.so)
==2658==    by 0xB37A83E: pa_context_new_with_proplist (in /usr/lib/x86_64-linux-gnu/libpulse.so.0.13.4)
==2658==    by 0x636516E: ??? (in /usr/lib/libopenal.so.1.13.0)
==2658==    by 0x6365674: ??? (in /usr/lib/libopenal.so.1.13.0)
==2658==    by 0x6367CD2: ??? (in /usr/lib/libopenal.so.1.13.0)
==2658==    by 0x62DE4E3: alcOpenDevice (in /usr/lib/libopenal.so.1.13.0)
==2658==    by 0x5BE423: CSoundManager::AlcInit() (SoundManager.cpp:297)
==2658==    by 0x5BE9FB: CSoundManager::CSoundManager() (SoundManager.cpp:274)

I tested SoundManager.2.patch​ and it fixes the first two, but there are also two new leaks:

==2658== 48 bytes in 1 blocks are definitely lost in loss record 93 of 270
==2658==    at 0x4C28B35: operator new(unsigned long) (vg_replace_malloc.c:261)
==2658==    by 0x5BEA0C: CSoundManager::CSoundManager() (SoundManager.cpp:275)
==2658==    by 0x5BED0C: CSoundManager::CreateSoundManager() (SoundManager.cpp:231)
==2658==    by 0x5A7289: Init(CmdLineArgs const&, int) (GameSetup.cpp:886)
==2658==    by 0x41D9BD: RunGameOrAtlas(int, char const**) (main.cpp:504)
==2658==    by 0x4136F6: main (main.cpp:551)

==2658== 1,024 bytes in 1 blocks are definitely lost in loss record 249 of 270
==2658==    at 0x4C2864B: operator new[](unsigned long) (vg_replace_malloc.c:305)
==2658==    by 0x5BE470: CSoundManager::AlcInit() (SoundManager.cpp:306)
==2658==    by 0x5BE9FB: CSoundManager::CSoundManager() (SoundManager.cpp:274)
==2658==    by 0x5BED0C: CSoundManager::CreateSoundManager() (SoundManager.cpp:231)
==2658==    by 0x5A7289: Init(CmdLineArgs const&, int) (GameSetup.cpp:886)
==2658==    by 0x41D9BD: RunGameOrAtlas(int, char const**) (main.cpp:504)
==2658==    by 0x4136F6: main (main.cpp:551)

[stwf]

I'll take this over and try to get rid of those past few this week. Thanks for all of the help on this...

[stwf]

I think this eliminates all leaks but I'm not using valGrind (can I on the Mac?) can someone confirm this?

[Cyrus Cousins]

Valgrind says we are leak free (well, at least as far as the sound manager goes). Nice!