why does removeSelf() on a display group not work in this short piece of code? (memory leak)

greg886 · August 16, 2013, 10:47pm

Why does this code “leak” in the case where some data is stored within a display group?

Based on the doco here http://docs.coronalabs.com/guide/media/displayObjects/index.html#remove shouldn’t the “group.removeSelf()” work in this case???

local useArrayInstead = false function createTestGroup() &nbsp; &nbsp; if (not useArrayInstead) then &nbsp; &nbsp; &nbsp; &nbsp; -- Case 1: Use a display group &nbsp; &nbsp; &nbsp; &nbsp; testArrayOrGroup = display.newGroup() &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-- LEAKS!!! &nbsp; &nbsp; else &nbsp; &nbsp; &nbsp; &nbsp; -- Case 2: Use an array &nbsp; &nbsp; &nbsp; &nbsp; testArrayOrGroup = {} &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-- Does NOT Leak &nbsp; &nbsp; end &nbsp; &nbsp;&nbsp; &nbsp; &nbsp; for i=1,100 do &nbsp; &nbsp; &nbsp; &nbsp; table.insert(testArrayOrGroup, {a = "asdf", b = "adsfasdf", fn = function() print("xxxx") end }) &nbsp; &nbsp; &nbsp; &nbsp; for j=1,100 do &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; table.insert(testArrayOrGroup[i], {a = "asdf", b = "adsfasdf"}) &nbsp; &nbsp; &nbsp; &nbsp; end &nbsp; &nbsp; end &nbsp; &nbsp;&nbsp; &nbsp; &nbsp; return testArrayOrGroup end function gcLuaLocalsTest() &nbsp; &nbsp; for i=1,10 do &nbsp; &nbsp; &nbsp; &nbsp; local myTestGroup = createTestGroup() &nbsp; &nbsp; &nbsp; &nbsp;&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; if not useArrayInstead then &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; myTestGroup:removeSelf() &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-- Does NOT work &nbsp; &nbsp; &nbsp; &nbsp; end &nbsp; &nbsp; &nbsp; &nbsp; myTestGroup = nil &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;-- Does nothing &nbsp; &nbsp; &nbsp; &nbsp;&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; collectgarbage() &nbsp; &nbsp; &nbsp; &nbsp; local memUsage\_str = string.format( "mem %.3fKB", collectgarbage( "count" ) ) &nbsp; &nbsp; &nbsp; &nbsp; print(i .. ", " .. memUsage\_str) &nbsp; &nbsp; end &nbsp; &nbsp;&nbsp; &nbsp; &nbsp; -- profiler.fullSnapshot() end gcLuaLocalsTest() &nbsp;

Console Output:

Copyright (C) 2009-2013 &nbsp;C o r o n a &nbsp; L a b s &nbsp; I n c . 2013-08-17 08:37:27.856 Corona Simulator[1580:707] &nbsp; &nbsp; Version: 2.0.0 2013-08-17 08:37:27.856 Corona Simulator[1580:707] &nbsp; &nbsp; Build: 2013.1178 2013-08-17 08:37:27.862 Corona Simulator[1580:707] The file sandbox for this project is located at the following folder: &nbsp; &nbsp; (/Users/Greg/Library/Application Support/Corona Simulator/memoryLeakTest-65AEC6564F32F1046AA9F1BABE8DDB7F) 2013-08-17 08:37:27.874 Corona Simulator[1580:707] 1, mem 1812.018KB 2013-08-17 08:37:27.879 Corona Simulator[1580:707] 2, mem 3446.236KB 2013-08-17 08:37:27.886 Corona Simulator[1580:707] 3, mem 5080.455KB 2013-08-17 08:37:27.893 Corona Simulator[1580:707] 4, mem 6714.674KB 2013-08-17 08:37:27.901 Corona Simulator[1580:707] 5, mem 8348.893KB 2013-08-17 08:37:27.910 Corona Simulator[1580:707] 6, mem 9983.111KB 2013-08-17 08:37:27.920 Corona Simulator[1580:707] 7, mem 11617.330KB 2013-08-17 08:37:27.932 Corona Simulator[1580:707] 8, mem 13251.799KB 2013-08-17 08:37:27.944 Corona Simulator[1580:707] 9, mem 14886.018KB 2013-08-17 08:37:27.957 Corona Simulator[1580:707] 10, mem 16520.236KB &nbsp;

rob · August 17, 2013, 3:20am

You should only be putting display objects inside a group. Looks like your insert tables into it and that’s probably an undefined behavior.

greg886 · August 17, 2013, 3:40am

oh…I’ve actually used the concept quite a bit of encapsulating object display and behavior in the same OO like file, which has helped the readability of my code. Am I potentially doing the wrong thing and getting lots of little memory leaks. The problem I have now, with a memory leak, has come about with a class where there is actually lots of data in it.

As an example I do have display type objects/components like this Rob. So strictly speaking should I be changing this approach? What about have the top level of a “display OO object” as an array, and store the data in the array in one property and the display objects in another property then?

-- Sample code, chopped down in size just to give an overview of the structure Blower = {} function Blower:new(name, x, y, ref, angle) &nbsp; &nbsp; &nbsp;local blower = display.newGroup() &nbsp;&nbsp;&nbsp;&nbsp;-- Data &nbsp;&nbsp;&nbsp;&nbsp;blower.data = { xxx }&nbsp;&nbsp;&nbsp;&nbsp;-- data stuff &nbsp;&nbsp;&nbsp;&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;function blower:setAngle(target, angle)&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;-- do stuff &nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp;-- Constants &nbsp; &nbsp;&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;-- Image &nbsp;&nbsp;&nbsp;&nbsp;local blowerImg = &nbsp;display.newImageRect(blower, "media/images/cannon.png", 95, &nbsp;37) &nbsp;&nbsp;&nbsp;&nbsp;blower.x, blower.y = x, y &nbsp;&nbsp;&nbsp;&nbsp;blower:setAngle(blower, angle) &nbsp;&nbsp;&nbsp;&nbsp;-- Listeners &nbsp;&nbsp;&nbsp;&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;function touchListener(event) &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;-- do stuff &nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp;blower:addEventListener("touch", touchListener) &nbsp; &nbsp; return blower end return Blower &nbsp;

greg886 · August 17, 2013, 3:49am

PS Can I add in 2nd question too. If I’m using my above-mentioned technique with Storyboard then I suppose the automatic memory handling of instances of my classes would not work too then? Ie normally storyboard should auto remove all the display objects and groups of display objects in scene.view, but if the display group also had other data/functions in it?

greg886 · August 17, 2013, 11:02am

Doing some more testing. I can still see leaking memory in the case that I create a simple display group, and then immediately do a removeSelf() on it.

Is this a bug in Corona? If not, what’s wrong with my code?

function log(i) &nbsp;&nbsp;&nbsp;&nbsp;collectgarbage() &nbsp;&nbsp;&nbsp;&nbsp;local mem = collectgarbage( "count" ) &nbsp;&nbsp;&nbsp;&nbsp;local memUsage\_str = string.format( "mem %.3fKB", &nbsp;mem) &nbsp;&nbsp;&nbsp;&nbsp;print(i .. ", " .. memUsage\_str) end log("Before") for i=1,1000 do &nbsp;&nbsp;&nbsp;&nbsp;local obj = display.newGroup() &nbsp;&nbsp;&nbsp;&nbsp;obj:removeSelf() &nbsp;&nbsp;&nbsp;&nbsp;obj = nil end log("After") &nbsp;

Output:

Copyright (C) 2009-2013 &nbsp;C o r o n a &nbsp; L a b s &nbsp; I n c . 2013-08-17 20:58:33.099 Corona Simulator[2371:707] &nbsp;&nbsp;&nbsp;&nbsp;Version: 2.0.0 2013-08-17 20:58:33.099 Corona Simulator[2371:707] &nbsp;&nbsp;&nbsp;&nbsp;Build: 2013.1178 2013-08-17 20:58:33.111 Corona Simulator[2371:707] Before, mem 177.182KB 2013-08-17 20:58:33.113 Corona Simulator[2371:707] After, mem 380.549KB &nbsp;

greg886 · August 17, 2013, 11:12am

PS

If I add some display objects under the group the memory leak jumps up again???

Copyright (C) 2009-2013 &nbsp;C o r o n a &nbsp; L a b s &nbsp; I n c . 2013-08-17 21:07:20.669 Corona Simulator[2371:707] &nbsp;&nbsp;&nbsp;&nbsp;Version: 2.0.0 2013-08-17 21:07:20.669 Corona Simulator[2371:707] &nbsp;&nbsp;&nbsp;&nbsp;Build: 2013.1178 ]2013-08-17 21:07:20.681 Corona Simulator[2371:707] Before, mem 177.469KB 2013-08-17 21:07:20.705 Corona Simulator[2371:707] After, mem 2328.375KB &nbsp;

function log(i) &nbsp;&nbsp;&nbsp;&nbsp;collectgarbage() &nbsp;&nbsp;&nbsp;&nbsp;local mem = collectgarbage( "count" ) &nbsp;&nbsp;&nbsp;&nbsp;local memUsage\_str = string.format( "mem %.3fKB", &nbsp;mem) &nbsp;&nbsp;&nbsp;&nbsp;print(i .. ", " .. memUsage\_str) end log("Before") for i=1,100 do &nbsp;&nbsp;&nbsp;&nbsp;local obj = display.newGroup() &nbsp;&nbsp;&nbsp;&nbsp;for j=1,100 do &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;local cir = display.newCircle(10,10, 10) &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;obj:insert(cir) &nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;obj:removeSelf() &nbsp;&nbsp;&nbsp;&nbsp;obj = nil end log("After") &nbsp;

greg886 · August 18, 2013, 9:42pm

@Rob / Corona - (bump) could I ask for some feedback on this please. I am trying to sort out a memory leak in my game but trying to work out (without having a profiler) what amount of leakage is coming from Corona display object leaks that I can’t fix myself?

rob · August 18, 2013, 11:56pm

function log(i) &nbsp;&nbsp;&nbsp; collectgarbage("collect") &nbsp;&nbsp;&nbsp; local mem = collectgarbage( "count" ) &nbsp;&nbsp;&nbsp; local memUsage\_str = string.format( "mem %.3fKB",&nbsp; mem) &nbsp;&nbsp;&nbsp; print(i .. ", " .. memUsage\_str) end log("Before") for i=1,100 do &nbsp;&nbsp;&nbsp; local obj = display.newGroup() &nbsp;&nbsp;&nbsp; for j=1,100 do &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; local cir = display.newCircle(10,10, 10) &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; obj:insert(cir) &nbsp;&nbsp;&nbsp; end &nbsp;&nbsp;&nbsp; obj:removeSelf() &nbsp;&nbsp;&nbsp; obj = nil &nbsp;&nbsp;&nbsp; collectgarbage("collect") end log("After") &nbsp; -- results 2013-08-18 19:47:34.600 Corona Simulator[27728:707] Before, mem 179.839KB 2013-08-18 19:47:34.699 Corona Simulator[27728:707] After, mem 186.341KB

First, your test isn’t probably a real world example. When you use these for loops to create a bunch of items then expect the memory to show correctly because garbage collection runs at odd times. In your case, it’s also a great example of how to create a memory leak.

Your outside for loop writes over obj every time. Even though you are calling removeSelf() and nil’ing it out, you create a new version before garbage collection has time to run. In the real world if you’re going to be spawning things like this you would put each obj in a table instead of writing over the existing code.

Once I told it to collect the garbage after each loop, that memory allocated is actually freed instead of the pointer getting written over before you can do anything about it.

You also had a bug with your collectgarbage() call. You need to specify "collect’ to tell it what to do. The docs say this is optional and it defaults to “collect” but either the docs or wrong or that feature is broken. I’ll report that to the engineers and let them sort out which is right or wrong.

greg886 · August 19, 2013, 12:08am

thanks Rob,

Some follow up questions if I may:

a) So I guess the question comes down to what is real world code. In my case the issue I have is a complex storyboard scene. When the user backs right out of the scene and enters in again I see memory creep/leak. So to try to understand what could be the issue I’ve written a little test app that loops (quickly) going into/outof that scene. Does this mean to re-create the same memory leak issue I should be some delay/pauses to simulate the time it takes for a user to click on BACK and then click on ENTER again to go back into the scene? i.e. in trying to find where the memory leak is I need to separate the “not real world code” memory leaks from the real ones I’m trying to track down I guess? Is say 0.5 second enough to let the garbage collector catchup? Am I on the right track re my thinking here?

b) Also would love your response to my first response to the thread re my technique if you could. It starts with “oh…I’ve actually used the concept quite a bit of encapsulating object display and …”

rob · August 19, 2013, 12:24am

Without seeing all of your code, it’s hard to tell what is leaking… if it’s leaking. An increase in memory usage isn’t necessarily a leak. Leak’s are lost pointers to code. If you could in your other scene as part of hitting the back button or as part of setting up that scene, call your collectgarbage(“collect”) call and see if that helps. It looks like you’re increasing by 2MB each time, which is pretty big, this likely an image that is in the 257-512px x 513-1024xpx range (could be multiple images) or perhaps a sound (or sounds) that you keep allocating each time your scene loads.

As to the “first response”, I’m not a big OOP person and if I’m honest, I don’t understand what you’re doing. While i guess you can technically shove any table into a display.newGroup(), its not designed for that and I can’t predict what system behavior will happen. I’m assuming that under the hood, we loop over all the items inserted into the group and call it’s :removeSelf() method and normal tables inserted won’t have that function so I don’t know how it goes about cleaning up that table cell.

If you want a table of objects, you should use a table and if you want a removeSelf() method, provide your own.

greg886 · August 19, 2013, 12:31am

thanks Rob - it’s leaking about 300k each time I re-enter the scene - tried the garbage collect but it didn’t help unfortunately - will keep reviewing

There’s no tool is there that can highlight where memory is “lost” so to speak (e.g. lost pointers to code)? (he asks expecting the answer no)

rob · August 19, 2013, 1:58am

If you’re building for iOS, you can build with a development profile and use the XCode “Instruments” tool to watch for leaks.

greg886 · August 19, 2013, 2:11am

ok thanks, but re using Xcode instruments:

* if there was a leak would it highlight the code/variable names so I can relate it back to my code?

* will it be any more accurate than the lua gabagecollector statements I’m using?

greg886 · August 19, 2013, 2:30am

PS, OMG Rob just found one small part of the problem. Look at this snippet. If the cleanup function is “above” the listener in the code it leaks, but if it “afterwards” it does not. So my “cleanup” function can’t really see finishedLevelListener being in scope I guess, BUT there is no error message to warn you to this.

Any tips/tricks re avoiding getting caught out like this? Is there a way to “confirm” when you remove a listener it really got removed? (plus my questions from last post re xcode if thats ok)

&nbsp;&nbsp;&nbsp;&nbsp;function stars:cleanup() &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;Runtime:removeEventListener ("finishedLevel", finishedLevelListener) &nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;local function finishedLevelListener(event) &nbsp; &nbsp; &nbsp; -- do stuff &nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp;Runtime:addEventListener ("finishedLevel", finishedLevelListener) &nbsp;

greg886 · August 19, 2013, 6:20am

ok made some improvement - can now programmatically trigger it to exit/re-enter the scene 100x in a row and the memory usage goes from: 1693kB to 1755kB only. Can I say this is good enough then and call it quits?

rob · August 19, 2013, 9:57pm

Actually, your code above wouldn’t necessarily flag an error, though it would be nice if it would. But here is what happens.

When the app’s code got compiled, finishedLevelListener is undefined. Therefore, it assumes you are looking for a global variable named finishedLevelListener and that value is nil. Your removeListener translates to:

Runtime:removeEventListener (“finishedLevel”, nil )

which is perfectly valid. Then later, finishedLevelListener gets defined and now has a value. Just for fun lets say for demo purposes the address/pointer to the function is now 0x102948348. You create the event listener:

Runtime:addEventListener (“finishedLevel”, 0x102948348)

Now when the removeEventListener runs, it looks for a “finishedLevel” event with a value of nil and it can’t find it, so it goes on about life.

XCode’s Instruments has no knowledge of Corona’s variables or line numbers, so it’s not going to give you a lot of information. You can watch the allocations and watch your memory go. But what you’re looking for with the Leaks tool are little spikes which indicate a chunk of memory that the app has lost a reference too and can’t be freed up. It’s normal for an app to grow in memory as long as the app retains its pointers to that memory.

I wouldn’t say its more or less accurate, it’s just a different way of looking at the information. I would say your app is probably okay with that much memory increase, though what’s it like with 1000? is it still increasing? Then you probably have a leak still. Though with that little of an increase, it’s likely to not crash your app.

greg886 · August 19, 2013, 10:03pm

thanks Rob - I think it would be well worth putting this gottcha in the pinned memory leak post - for the moment I went through my code and put an assert for the listener right before each removeEventListener just in case. I guess I had been treating my OO like layout of LUA too much like real OO

[oh - separately - if you happened to know an answer offhand to this question that would be nice ]

rob · August 19, 2013, 10:16pm

There is a module called “strict.lua” floating around. We hope to do a blog post on it at some point that you can include in your main.lua and it blows up at you when you try to access undefined globals and such. I’ve not used it yet, but it might have caught this.

rob · August 17, 2013, 3:20am

You should only be putting display objects inside a group. Looks like your insert tables into it and that’s probably an undefined behavior.

greg886 · August 17, 2013, 3:40am

oh…I’ve actually used the concept quite a bit of encapsulating object display and behavior in the same OO like file, which has helped the readability of my code. Am I potentially doing the wrong thing and getting lots of little memory leaks. The problem I have now, with a memory leak, has come about with a class where there is actually lots of data in it.

As an example I do have display type objects/components like this Rob. So strictly speaking should I be changing this approach? What about have the top level of a “display OO object” as an array, and store the data in the array in one property and the display objects in another property then?

-- Sample code, chopped down in size just to give an overview of the structure Blower = {} function Blower:new(name, x, y, ref, angle) &nbsp; &nbsp; &nbsp;local blower = display.newGroup() &nbsp;&nbsp;&nbsp;&nbsp;-- Data &nbsp;&nbsp;&nbsp;&nbsp;blower.data = { xxx }&nbsp;&nbsp;&nbsp;&nbsp;-- data stuff &nbsp;&nbsp;&nbsp;&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;function blower:setAngle(target, angle)&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;-- do stuff &nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp;-- Constants &nbsp; &nbsp;&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;-- Image &nbsp;&nbsp;&nbsp;&nbsp;local blowerImg = &nbsp;display.newImageRect(blower, "media/images/cannon.png", 95, &nbsp;37) &nbsp;&nbsp;&nbsp;&nbsp;blower.x, blower.y = x, y &nbsp;&nbsp;&nbsp;&nbsp;blower:setAngle(blower, angle) &nbsp;&nbsp;&nbsp;&nbsp;-- Listeners &nbsp;&nbsp;&nbsp;&nbsp; &nbsp;&nbsp;&nbsp;&nbsp;function touchListener(event) &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;-- do stuff &nbsp;&nbsp;&nbsp;&nbsp;end &nbsp;&nbsp;&nbsp;&nbsp;blower:addEventListener("touch", touchListener) &nbsp; &nbsp; return blower end return Blower &nbsp;