phaser icon indicating copy to clipboard operation
phaser copied to clipboard

DisplayList causes infinite loop that is hard to track down.

Open hunkydoryrepair opened this issue 10 months ago • 3 comments

Version

  • Phaser Version: 3.60.0
  • Operating system: Windows/all

Description

The DisplayList.shutdown code changed between 3.55.x and 3.60.0, creating a very serious bug when a GameObject does not call the base class destroy method. In 3.55.x, calling super.destroy() was essentially optional, since it didn't do anything in most cases. Therefore, there are likely a lot of projects that have this problem and will experience difficulty moving to 3.60.0.

this comes from the suspicious looking code in shutdown:

while (list.length) {
   list[0].remove();
}

While this works as long as every removed object actually removes itself from the display list, when it does not, this goes forever.

Example Test Code

class MyObject extends Container {
    destroy() {
    } 
}

Adding MyObject as above to a scene, and the browser becomes unresponsive when destroying the scene.

Additional Information

Some advice in the ChangeLog for 3.60.0 for this situation would be helpful. I'm not sure which Javascript version Phaser targets, but there are a lot safer methods to iterate on an array. Using a forEach could handle the iteration whether the objects remove themselves or not.

hunkydoryrepair avatar Aug 23 '23 00:08 hunkydoryrepair

Calling destroy is vital and must be done. Replacing it with an empty function will completely break the internals of Phaser.

photonstorm avatar Aug 23 '23 08:08 photonstorm

I am currently having a similar serious issue with this code block forming an infinite loop.

   /**
   * The Scene that owns this plugin is shutting down.
   *
   * We need to kill and reset all internal properties as well as stop listening to Scene events.
   *
   * @method Phaser.GameObjects.DisplayList#shutdown
   * @private
   * @since 3.0.0
   */
  shutdown: function ()
  {
      var list = this.list;

      while (list.length)
      {
          list[0].destroy(true);
      }

      this.events.off(SceneEvents.SHUTDOWN, this.shutdown, this);
  },

because the gameObject i have somehow does not have a scene attached anymore it gets filtered out in the destroy function:

        //  This Game Object has already been destroyed
        if (!this.scene || this.ignoreDestroy)
        {
            return;
        }

and therefore never gets removed from the list. So if the above mentioned list contains any destroyed objects, or objects where the scene is undefined, it will cause an infinite loop and freeze the browser. Is there any way to handle this?

frankakn7 avatar Oct 02 '23 13:10 frankakn7

@frankakn7 find out where in your code you are destroying the Game Objects, or incorrectly overriding the destroy method (as in the original issue), or if you've got any Game Objects set to ignoreDestroy. One of the first thing the destroy method does is:

        this.removeFromDisplayList();
        this.removeFromUpdateList();

As long as this completes, they're no longer on this list. There's no reason for this to never complete unless the Game Object has been modified in some way to prevent it.

photonstorm avatar Oct 02 '23 14:10 photonstorm

Going to close this, as destroy should never be replaced in this way and is documented as such.

photonstorm avatar Jan 31 '24 18:01 photonstorm