Possible memory leak in Phaser.Time

[glantucan]

I am a little bit confused about how timers work in Phaser:

I understand that the Phaser.Time instance accessible from game.time is responsible for all the internal timing of the engine and also for the timed events we want to add in our games.

But I don't understand the following (even after reading the source code)

• First, the name of the main game timer property game.time.events. It's odd because is a Timer object, not a signal. Shouldn't it be called mainTimer or something like that? I understand it might have this name for historical reasons and a change would break working code, but I'm curious
• If we want to create new timers we have game.time.add and game.time.create. We can destroy these timers form it's own API (Timer.destroy()) But that doesn't seem to delete them from the _timers array used by the Time instance to update them. We do have Time.removeAll(), but shouldn't we have Time.remove(timer)? . Otherwise we would have a potential memory leak, right?. 
  I saw in the source code that the timers are automatically removed from this list in the Time.update() function if they have no more events to be dispatched, but only if their autoDestroy property is set to true. What if you don't set this property to true? I'm sure I don't want to, because I like to reuse my timers. Am I missing something?
   

Thanks in advance

Óscar

[VitaZheltyakov]

Any code involving execution after some time there is a potential memory leak.
To completely remove a timer you need to completely remove the "content", but it is not always possible.

[glantucan]

9 hours ago, VitaZheltyakov said:

Any code involving execution after some time there is a potential memory leak.
To completely remove a timer you need to completely remove the "content", but it is not always possible.

Thanks for the asnwer, but sorry, I don't quite understand what you mean by 'content'. And yes, there's always potential for memory leaks if one doesn't clean up thoroughly. But that's not what I meant.

I seems to me that the API is either confusing or there is a bug, because when you do a yourTimer.destroy() that function doesn't clean the reference to the timer in game.time._timers.

If one trust the destroy() function it will produce memory leaks because the garbage collector won't be able to collect that timer even if one would have cleaned up all its own references.

You can workaround this by setting  yourTimer.autoDestroy to true. That will get rid of the reference in the next game.time.update(), but that is not properly documented and seems obscure to me.

I may be wrong, or mistaken on the way to use timers, that's the reason I'm asking here before reporting it as a bug.

 

 

 

[VitaZheltyakov]

13 hours ago, glantucan said:

Thanks for the asnwer, but sorry, I don't quite understand what you mean by 'content'. And yes, there's always potential for memory leaks if one doesn't clean up thoroughly. But that's not what I meant.

I seems to me that the API is either confusing or there is a bug, because when you do a yourTimer.destroy() that function doesn't clean the reference to the timer in game.time._timers.

If one trust the destroy() function it will produce memory leaks because the garbage collector won't be able to collect that timer even if one would have cleaned up all its own references.

You can workaround this by setting  yourTimer.autoDestroy to true. That will get rid of the reference in the next game.time.update(), but that is not properly documented and seems obscure to me.

I may be wrong, or mistaken on the way to use timers, that's the reason I'm asking here before reporting it as a bug.

 

 

 

It is not bug. It is Java Script

[mattstyles]

@glantucan I don't understand the answer given either.

My understanding of GC is the same as yours, if any refs remain than the object is not a candidate for GC.

From your description it sounds like there could be a leak, and one that maybe could be avoided by the framework (these things are traditionally punted on to the consumer, and for good reason, i.e. you assign your references so you must destroy them when you're done). How often do you create and destroy timers though?

Simple way to test just keep creating timers and check your memory consumption in your browser.

Also, might want to check the github repo for related discussions and/or open an issue there.

[glantucan]

On 6/6/2016 at 11:21 PM, VitaZheltyakov said:

It is not bug. It is Java Script

Ok, I'll take as a joke

 

[glantucan]

22 hours ago, mattstyles said:

@glantucan I don't understand the answer given either.

My understanding of GC is the same as yours, if any refs remain than the object is not a candidate for GC.

From your description it sounds like there could be a leak, and one that maybe could be avoided by the framework (these things are traditionally punted on to the consumer, and for good reason, i.e. you assign your references so you must destroy them when you're done). How often do you create and destroy timers though?

Simple way to test just keep creating timers and check your memory consumption in your browser.

Also, might want to check the github repo for related discussions and/or open an issue there.

Hi again,

ok, sorry, I tried my best to explain without going too deep into the details. I thing the piece that's missing is that the _timers array is not one I created, is just the way Phaser keeps a reference on the Time instance to all timer instances, so it can update them. Your timer gets pushed to that array when you create one using game.time.create()

I did test it on the game I'm working on and I can see that the references are not deleted from the _timers array on the Time instance of the game after you use their destroy() function which I call just before I delete all my own references to it. That's the point. in general when you call destroy() on any Phaser object it takes care of all internal references. In this case it does not.

I realized on this because one of my loop timers kept firing after I used destroy(), which I forgot to mention in my first post and could be more of a bug than the memory leak itself. For this, though, the workaround is easy and evident, stop() the timer before detroying it.

I will open an issue on the repo, but I wanted to follow the intructions there: Ask in the forum first

You see, maybe it's the intended use for the API: you'll have to set the autodestroy flag if you want internal references to be deleted. But it seems weird to me though.

[mattstyles]

Yeah, continuing to fire after a destroy violates the principle of least surprise (at least for me it does), whilst I can probably see why manually setting the autoDestroy flag is maybe preferred (I'm guessing defaulting to false was a considered decision) I'm not clear why destroy() would not call stop() before tidying up.

@rich would be able to shed some light quickly on why the _timer ref isn't removed.