Massive memory leaks

Sorry forgot to format the file properly. When you look at the stderr of valgrind, best directly scroll down to the end as the memory leaks are ordered ascending by size of leaked memory

Maybe i misinterpret the output but isn't it less than one mb total definitively lost? I hardly all that massive these days.

Well you missinterpret it in that way, that a normal game takes longer than 3-5 minutes and the longer a game takes, the more buildings, workers, wares and the like are produced. Just keep in mind: every field, every planted tree, every bread animal (gamekeeper) and every newly created workers (maybe even ware) seems to lead to a small memory leak. Further the game was started on a 64x64 map with just one AI opponent - that run took already enough time when running widelands in valgrind, running a bigger map would take even more time until the game is finally loaded.

However you are right in that way, that these numbers do not say that much - I used the dedicated mode, that does not show possible memory leaks in graphic code and I ran valgrind with --show-possibly-lost=no , so only definitly lost memory is shown in the list. The list of "possibly lost memory" + "definitely lost memory" is muuuuch longer ;)

However to show some numbers that look more realistic and show the full "memory leak" problem (so summary of all leaks ;) ):

I started a new game (singleplayer) on the map "Atoll.wmf" (still small used terrain because of the big water size) with three AI and me. I saved the game after 20 minutes and closed widelands.
After a restart of Widelands, I reloaded the game immediatly closed it again, reloaded it, closed it and reloaded it a third time.
Here is the memory usage of Widelands (output of the taskmanager):

before loading the first time: 10.4MB
After 1st loading (in game): 162.5MB
After stoping the game: 157.9MB
After 2nd loading (in game): 304.2MB
After stoping the game: 298.5MB
After 3rd loading (in game): 444.4MB
After stoping the game: 437.8MB

And again - this is a 20 minutes game, I was far from having a complete economy and had no enemy contact yet.

Okay, I agree that fixing memory leaks when loading and creating objects should be properly done. I disagree that the solution should be a catch all list - instead we need to find each individual leak i fear. One solution could be to make a small lua map that creates a few buildings, let the game run for a few minutes and terminates the game - this could then be run with valgrind in an automatic fashion and provide benchmark.

Your top analysis is flawed: when a program free()s memory, it will still be listed as used by the program until the OS reclaims it. Gauging 'real' memory usage without a malloc() library is actually pretty hard. you could try the smem tool under linux. See also the top rated answer to http://stackoverflow.com/questions/4802481/how-to-see-top-processes-by-actual-memory-usage.

So I've been trying to narrow it down with memory leak checkers, that did turn up some positives, but nothing major, I've been fixing those in my branch: lp:~csirkeee/widelands/memory-leaks-2

But then, playing around with the game, paying close attention to the memory consumption, I found a pretty minimal case: whenever I clicked on "new map" in the editor's menu, there was like 8 MB of new memory used. (Note, that I didn't even create a new map, just opened the dialog for it!)

Tracing into it, what I found was that
1) Opening that dialog loads in all the world config files, just so it can find out the names of the worlds
2) While loading in the config files it also loads in all the sounds associated with the world
3) This, through Sound_Handler::load_one_fx() loads the sounds into memory, that only get deleted when the sound handler is destroyed, when quitting the game (so strictly speaking it's not a memory leak, the detectors didn't find it).

Basically, Sound_Handler loads the same sound files over and over and over. The solution I guess would be to have the sound handler use some kind of cache too, like with images. I don't know exactly how big a dent this will be from our memory consumption, I think it's significant.

If noone else wants to do a sound cache, I might get on it soon.

Cool! let us know when your branch is ready for merging.

I like the idea with the cache. This could be more clever than the image caches - for example music files do not need to stay in memory after they are no longer played, because they are so long and a small delay loading them is not too bad. You can model this class after SurfaceCache or ImageCache that is already in the code base.

I've fixed quite a few memory leaks recently, could you check behavior now? For me, running normal gameplay didn't turn up anything with memory checker now. If you provide the command line with which you ran the dedicated server, I could also try that. (One of the leaks in your stderr file seems to be from network code that I'm not sure I've tested.)

If it seems good enough for you, we could close the bug. I think we probably won't be completely memory leak free soon, but that's okay.

I've started to look a bit at valgrind now, and I got the following report from latest trunk:

==28385== HEAP SUMMARY:
==28385== in use at exit: 82,630 bytes in 896 blocks
==28385== total heap usage: 3,218,096 allocs, 3,217,200 frees, 1,932,028,089 bytes allocated
==28385==
==28385== LEAK SUMMARY:
==28385== definitely lost: 806 bytes in 8 blocks
==28385== indirectly lost: 0 bytes in 0 blocks
==28385== possibly lost: 0 bytes in 0 blocks
==28385== still reachable: 81,824 bytes in 888 blocks
==28385== suppressed: 0 bytes in 0 blocks
==28385== Rerun with --leak-check=full to see details of leaked memory

This is from loading a game, playing it for a couple of minutes, then loading another game to let it run. So probably not as intensive as Nasenbaer's check, nor did I use the dedicated server. Though, it still seems like things have improved. (I get the roughly 800 bytes in "definitely lost" from just starting the game and immediately clicking Exit Game too). Could someone with the knowhow take a second look at this and the various other reports on leakage to check if they are still valid?

They probably are - but leaking 82k of memory is not that bad (especially since we allocate ~2 gigs). I am not too concerned about these, though of course they are bugs and should be fixed.

I am reducing the priority of this and retarget for investigation with b19.

I just checked build18, using a 18h long replay on a 512x512 world with valgrind, and I did not find any leak, actually.

From what I can see, the leaks come from some libraries that we use, not from widelands itself.

So I think that this bug can be closed now.

great. Thanks for checking this - must have taken a full day as valgrind is soooo slow.

well, it depends on the valgrind tool that you are using. Massif (the heap profiler) is rather fast. It's not really playable, but I can hope for 3fps at 1x. The standard tool (used for memleaks) is rather equivalent. But the callgrind tool, the execution time profiler, takes really for ages.

Fixed in build19-rc1.