mspraggs / potentia

Southampton Game Jam 2015
0 stars 0 forks source link

Memory leaks #37

Closed mspraggs closed 9 years ago

mspraggs commented 9 years ago

I just ran the game, compiled from the latest source, through valgrind's memory checker. I'm getting a lot of memory leaks coming from the graphics code. I've attached the output from valgrind. It shows the call trace for each call to malloc that results in a memory leak. I think we just need to make sure there's proper cleanup/deallocation code in the Window class to make sure all the relevant SDL/GL resources are correctly deleted when the window is closed.

Also, looking at how the Game and World handle the vector of Objects, I'm wondering whether it's worth using smart pointers to handle memory allocation, simply because that way we can be sure all the Objects will be garbage-collected properly.

EDIT: I wanted to upload the leak log, but GitHub doesn't do plain text files, so I'll email.

mspraggs commented 9 years ago

Some of the lines in the code where memory is allocated but not deallocated:

Window.cpp:28 Window.cpp:52 Window.cpp:78 Window.cpp:92 Window.cpp:116 Window.cpp:131 Game.cpp:16 (probably refers to a call within Window) GraphicPool.hpp:45 GraphicPool.hpp:117 Prop.hpp:16

This is by no means an exhaustive list. When I have more time I'll find some way to extract all relevant file/line numbers and remove duplicates so we have a complete list.

DivFord commented 9 years ago

I don't think the Destructor for Window ever gets called. We maybe need a Quit function before we return run to main.

mspraggs commented 9 years ago

Well... I mean the destructor should get called at some point. The compiler will create a default one if we don't give it something, and that will be called at some point. I think the Window instance is static though, so it'll probably be called when the program terminates. In any case, the longer the program is open, the more Window does stuff, and the more stuff gets left allocated on the heap.

Ideally for every call that allocates on the heap, there must be a corresponding call deallocate that memory.

Fyll commented 9 years ago

The destructor of a singleton class is called automatically when the program ends, so that's not a problem.

Also, on the smart pointer front, feel free to change them if need be, but I'm pretty sure everything's deallocated properly at the moment, and I have no idea how to use smart pointers (and think they look messy).

Hmm. I know that SDL_GL_SwapWindow() cases a leak, but I've never been able to fix it.

mspraggs commented 9 years ago

Reading up on things it appears you're somewhat at the mercy of SDL/GL when it comes to the graphics memory leaking, which I suspect may be what's happening in our code.

Valgrind also points to calls to newRoom when locating memory leaks, so I'm wondering whether exploiting smart pointers will save us a lot of bother further down the line. Looking at the code it shouldn't be too difficult to swap in std::unique_ptr or similar.

Fyll commented 9 years ago

Fair enough. We're going to have to be at the mercy of something for memory leaks when it comes to graphics (unless you want me to switch to X, which really ought to be leak free).

Hm. I thought I'd cleaned them all up properly. Ah well, smart pointers it is then (can you do this, I don't really know how to use them).

DivFord commented 9 years ago

I've got it to compile in Xcode (by ripping out audio completely) but when I run it I get a bad access exception at Window.cpp, line 145.

EDIT: The bad access was because I hadn't linked the graphics in properly. Derp.

Also, my compiler kindly pointed out that the if(surface) in GraphicPool.hpp, line 93 means that tex can be returned without being initialized. It seems upset about that.

Fyll commented 9 years ago

I meant use X11 to handle the window instead of SDL. The main drawbacks would be that it only has an ugly font (I can't open TTF's in it), and, more importantly, I only know how to open .bmp's (so no nice clean .png's).

DivFord commented 9 years ago

I got that. Getting it to run in Xcode was my attempt to move towards a deployable build instead of just source code. It's in this thread because I thought the compiler error might be relevant.

mspraggs commented 9 years ago

Now I've introduced smart pointers, and there's no longer any dynamic allocation in our code, I'm going to assume all memory leaks, if there are any, are coming from SDL. I've also done a brief bit of research, and found this: http://stackoverflow.com/questions/1997171/why-does-valgrind-say-basic-sdl-program-is-leaking-memory. Hence I'm closing this.