Memory leak with List

Monkey Forums/Monkey Programming/Memory leak with List

Kauffy(Posted 2012) [#1]
I have an Enemy class.

When creating a new Enemy, I call:
Local enemy:Enemy = New Enemy()


Inside Enemy.New, I call a class function that adds Enemy to a list, which is maintained within the class as a global.

At the start of a wave, I allocate a bunch of enemies using a series of calls like the above
Local enemy:Enemy

enemy = New Enemy()
enemy = New Enemy()
enemy = New Enemy()
enemy = New Enemy()
enemy = New Enemy()
.
.
enemy = New Enemy()


Any time an enemy is destroyed, it is .Removed() from this list-- as far as I can tell, other than one lingering reference in that recycled enemy variable, the only reference to the enemy is in the Enemy class master list.

At the end of a wave, I do a call to this Enemy.List.Clear() just to make sure they're all wiped out.

For some reason, though, I am getting a leak-- I never see any memory deallocated from my process. Is this a matter of GC never happening? Or is there something I'm doing dumbly that is creating references I'm not aware of?

Here is the enemy class code:
Class Enemy
	Global enemyListAll:List<Enemy> = New List<Enemy>
	
	Field tank:Tank 'Summary: Contains the tank object for this Enemy.
	Field targType:Int 'Summary: Contains the type of objective this enemy has currently. 1=Get Booty, 2=Escape, 3=Kill a Player
	Field targIndex:Int 'Summary: Combined with targType, contains a parameter. If targType=1, e.g., then this contains Index of the booty object in the booty array. This method is deprecated.
	Field target:ftObject 'Summary: An ftObject containing the target we're after. This simplifies some things. This is the new method.
	Field node:ftObject 'Summary: For pathfinding, where is the destination I am heading for?
	Field nodeAge:Int 'Summary: Timestamp of when the node was created. Aged nodes are probably ineffective nodes. If a node gets too stale, we want to re-node.
	Field moveMode:Int 'Summary: For sub-behavior withing pathfinding. 0 is normal, fast as I can moving toward node/target. 1 is something else.

	'Summary: Adds an Enemy object into the current enemy list.
	Function AddToAll:Int(newEnemy:Enemy)
		If enemyListAll.Contains(newEnemy) = False Then enemyListAll.AddLast(newEnemy)
		Return 0		
	End Function

	'Summary: Removes an Enemy object from the current enemy list.
	Function RemoveFromAll:Int(remEnemy:Enemy)
		If enemyListAll.Contains(remEnemy) = True Then enemyListAll.Remove(remEnemy)
#IF CONFIG="debug"
		Print "Enemy.RemoveFromAll entered."
		Print "There are " + enemyListAll.Count() + " enemies total."
#ENDIF
		Return 0
	End Function

	'Summary: Declare a new enemy "player" with skill of level (an int from 0 to, like 10). Currently, level is also used to determine the type of tank.
	Method New(level:Int)
		'Declare a new tank
		'Set an objective.
		Self.tank = New Tank(False, level)
		Self.targType = TRGGETBOOTY '##FIXME: Don't set the objective this way
		Self.target = Null
		Self.tank.gob.dataObj = Self
		Self.node = eng.CreateCircle(5, 0.0, 0.0)
		Self.node.SetColor(10, 255, 10)
		Self.node.SetVisible(False) 'If you want nodes visible for AI path debugging purposes, set this to True.
		Self.moveMode = MMNORMAL
		Enemy.AddToAll(Self)
	End Method

	'Summary: If I'm killed, score points and remove me and my tank from the game.
	Method Die:Void()
		Enemy.RemoveFromAll(Self)
		Self.tank.Die()
	End Method
	
	'Summary: If I've made it off the edge of the screen, remove me and my tank from the game, but don't give the player(s) the satisfaction of the points.
	Method Escape:Void()
		Enemy.RemoveFromAll(Self)
		Self.tank.Escape()
	End Method

End Class





muddy_shoes(Posted 2012) [#2]
Garbage collector behaviour varies but very few collectors will do anything until certain trigger points have been reached, normally in terms of memory allocated or available. It's also possible that an allocator will work with an internally managed block of memory so you won't see usage reduce by viewing a process externally.


Kauffy(Posted 2012) [#3]
I've got a stress test mode where I launch like 40 enemies (usual level will be 3-6, I think) and just let them come in and steal the loot. When they've stolen all of it, the wave resets-- so it just keeps launching waves until all the loot is gone.

After about five minutes or so, the frame rate begins to drop and will decline noticeably from there. If I watch the process, I only ever see it add memory-- never dealloc any (which strikes me as odd).


muddy_shoes(Posted 2012) [#4]
Which platform are you talking about? If it's one with a native GC then a GC defect seems unlikely. GLFW or iOS use the BRL homegrown collector, which is more likely to have bugs but something so blatant would probably have bit others before now.

Presumably your test runs show those Print statements saying that enemyListAll is actually being emptied? Have you tried cutting down the test so that you do nothing other than add and remove the tanks from the list rather than running your entire game? It seems probable that your problem is elsewhere.


Kauffy(Posted 2012) [#5]
Agreed-- unfortunately, this stuff was late-added, so isolating it is a little more work. It also appears to be more a conditional problem (as in, it shows up only under certain circumstances).

This is in GLFW.

I have also found a seeming problem where my List.Remove code checks for the existence of the member to be removed before attempting to remove it, though it often reports the member to be removed isn't actually in the list (which is why I had to go to a List.Clear strategy between waves).


MikeHart(Posted 2012) [#6]
not sure if that helps, but NULL out your ftObject fields and the tank field from you node when you remove it. The object holds references to itself, so it COULD be, that it lingers in memory on its own.


Kauffy(Posted 2012) [#7]
Yeah, I think I'm going to have to write something like a pervasive destructor.

It appears that my Enemys, even though their list has been blanked, still persist. Even as I wipe out the list, not only do they not disappear, they continue doing their thing!


Kauffy(Posted 2012) [#8]
Okay, so I wrote a .Finalize method for my Enemy class, my Tank class, and my Booty class.

The Enemy finalizer makes sure the Tank finalizer gets called, and then removes itself from the master Enemy list.

The Tank finalizer reaches down into the gob (ftObject) and Nulls its DataObj field (which is a reference back to the Enemy object).
It also sets the gob's deleted field to True, though it seems like there should be another way to do this. The Tank finalizer also sets all of its own reference Fields to Null to really make sure it's wiped out.

The Booty class does similar to the Tank class finalizer with reference to the gob.

This broke a bunch of my code (properly) because I had ways to call objects to update themselves after they were going out of the game and it did throw an error (where it did not before).

Unfortunately, the leak still persists.

Is there a proper way to get rid of an ftObject? I can remove all of my references to it, of course, but I imagine ftEngine maintains its own references to all the objects.

EDIT: I went and had a look at the code in ftObject and replaced my hand-setting of .deleted to True with the .Remove method.

Also, the leak is not just memory, but CPU-- so whatever is not getting deallocated is also still being "processed".

EDIT #2: I am getting calls back to OnObjectUpdate from ftObjects that have .deleted set to True. Is there a case where that is supposed to happen? I had to add a check for this because these calls trigger Update calls to my parent objects (Tank or Booty), which are deleted already.


MikeHart(Posted 2012) [#9]
You use your own way of handling ftObjects so you need to make sure that a deleted object isn't updated. And so no OnObjectUpdate is called. Look at ftLAyer.Update to see how I did it there.

EDIT: Actually the first lines in ftObject.Update should take care if this. But the call is positioned wrongly. You found a bug. :-)


MikeHart(Posted 2012) [#10]
Change the end of ftObject.Update to this:

[monkeycode] If Self.isWrappingX Then Self.WrapScreenX()
If Self.isWrappingY Then Self.WrapScreenY()

For Local child := Eachin childObjList
If child.isActive Then child.Update(delta)
Next
For Local trans:ftTrans = Eachin transitionList
trans.Update()
Next
For Local timer:ftTimer = Eachin timerList
timer.Update()
Next
If Self.onUpdateEvent = True Then engine.OnObjectUpdate(Self)
End
CleanupLists()

End[/monkeycode]


Kauffy(Posted 2012) [#11]
Testing that fix now!

The way I am using ftObjects shouldn't be that odd-- I am creating my objects as wrappers which contain ftObjects for their physical/graphical representation. Then I'm using the ftObject's data object to point back to the parent.

This way, I can rely on ftEngine's callbacks-- so when OnObjectUpdate is called, I look at what type of object it is (be reading CollGroup) and calling my own Update on the Cast(ftObject.GetDataObject) for the class. It seems to work very well. :)

[EDIT]
Leak situation seems improved quite a bit, but still leaking.

I also found that there is an ftObject I am using in Enemy that wasn't being Removed()-- just the reference set to Null. Without calling Remove, ftEngine would have no way to know I was done with the object. I will add some diagnostic stuff into ftE and see where I am going wrong-- there must be something I'm allocating and not properly freeing.

[EDIT #2]
I added a Print to ftObject.SetLayer to print the count of objects in the list-- it keeps growing. At this point, I am only creating objects in one Layer (I had to add a SetDefaultLayer call just to make sure).

[EDIT #3]
I'm seeing that the Remove code is being called and .deleted is being set, though I do not see anyplace that .deleted is actually being used to (really) Remove an object.

When CleanupLists in ftObject is called, it cleans up the transition and timer lists.

When ftObject.Remove is called, it only sets the .deleted flag.

Then, in cftLayer.CleanupLists it looks like it's going to clean it up-- it checks to see if the object has its .deleted flag set and, if it does, it calls .Remove-- except this isn't the real monkey List.Remove, this is the cftObject.Remove which doesn't actually Remove the object-- it just sets the .deleted flag.

Does that make sense?

[EDIT #4]
I just looked through it again-- it seems the ftObject.Remove code does remove the object from the Layer list-- but is that the only place the object is referenced? It would seem like it couldn't be, since the layer can be Null, right?


MikeHart(Posted 2012) [#12]
Yes, ftLayer.CleanupLists should actually remove it from the parents list and the layer list it is assigned to it. Do you assign it to 2 layers somehow? The object itself has transition lists, timer lists and children lists. I will add some extra nulling of object references there in the next version to make sure EVERYTHING is nulled and there is no chance for the missbehaviour of the GC to work its evil way.


MikeHart(Posted 2012) [#13]
I need to do some christmas duty today but later I try to replicate your problem. What do you do with the objects. Just creating and removing or do you have transitions, childs and timers also involved?


Kauffy(Posted 2012) [#14]
The problematic objects only sometimes have children (which are removed) and I'm not using any transitions or timers for these objects. The are assigned to only layer, as far as I can tell. I assume (without looking) that SetLayer would move an object from one layer to another-- not just create a new reference to it in the new layer.

The only issue I potentially had like this was before I set a default layer I was creating my objects and then explicitly assigning to the gameplay layer. Those explicit assignments are still there for all objects, and now I have set the default layer to.

I am dumping out a lot of Print to track down where the issue lies-- I want to be sure I'm not somehow creating additional objects. It has the appearance of it, but I can't tell why it would happen.

Thanks for looking into it.


MikeHart(Posted 2012) [#15]
I think you store each object in several layers. At least from your description it sounds like you do. I just made a stresstest which creates 100 objects with 3 childs each frame and deletes them too. So far I can't see any slow down.


Kauffy(Posted 2012) [#16]
If an object is created to a default layer that is not the intended layer, does a second call to SetLayer duplicate the object or reassign it? In other words, does it remain in the original default layer AND in the SetLayer'd layer?

I have been cleaning and rearranging all day today and haven't given this a moment's thought. Looking forward to getting back to it with fresh eyes. :)


MikeHart(Posted 2012) [#17]
SetLayer is surpose to remove the object from the current layers objectlist and add it to the one you had specify in the SetLayer call.


Kauffy(Posted 2012) [#18]
Well, that was a whole lot of goose chase, but a very useful learning exercise for me.

The leak was coming from several places-- I just wasn't managing my objects properly by either Monkey or ftEngine. Adding Finalizers to my major classes that explicitly dismantled their child objects (my child type, not ftE) and then making sure all the calls got funneled through them fixed a huge amount of it... the last little bit though is embarrassing:

I actually wrote in a memory leak knowing at the time I was doing so, put a comment on it saying so, along with a ##FIXME flag and then forgot that I did that! It was a solution for AI pathfinding to create a ftObject for targeting purposes-- so a new ftObject was being created every time the code went through and it wasn't being released.

It made the AI work, but left a ton of extra circles around. I have now added the code to the Finalizer to clean these up.

Thanks for your help and attention-- and at least we found one ftE bug out of it, right?

In you're curious, each of my enemy tanks has two other ftObjects attached to it. They're just created as (usually) invisible circles. One is for the Enemy's target object IF where the Enemy is headed is not another type of object-- for example, if the enemy is not going to steal loot or kill the player, then I create an off-screen circle for the enemy to path toward for escape.

The other object is an AI pathing node that is generated 1/3rd of the way between the enemy and whatever its target is. This way, the AI driving is much curvier and more interesting. It looks quite beautiful.

This just seemed an easier solution to be able to use ftEngine's distance/direction, etc. between objects for the AI.


zoqfotpik(Posted 2012) [#19]
Perhaps I'm old fashioned but I almost never run into problems with memory leaks and the reason is the fact that I typically use re-usable object pools instead of letting the GC handle things for me. This takes some extra work but in my opinion it's worthwhile.


MikeHart(Posted 2012) [#20]
On mobile devices you should reuse as much as possible anyway.


Kauffy(Posted 2012) [#21]
The game I'm working on was intended as a learning opportunity-- and this looks like just the kind of thing it's helpful to learn.

Fortunately, I only wanted to run this game in Windows (and possibly Xbox), but my next project will definitely be mobile.