Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PointerEventReceiver memory leak #3356

Open
blakearoberts opened this issue Feb 1, 2025 · 1 comment
Open

PointerEventReceiver memory leak #3356

blakearoberts opened this issue Feb 1, 2025 · 1 comment

Comments

@blakearoberts
Copy link
Contributor

Steps to Reproduce

  1. Instantiate and start an engine in any configuration.
  2. Add and initialize any scene that removes the default PointerSystem (e.g. this.world.remove(this.world.get(PointerSystem))).
  3. Move, click, or scroll your mouse.

The culprit of the issue is a combination of these two pieces of code:

  1. PointerEventReceiver appends pointer events to internal arrays that are cleared via an external call of its clear method.
  2. PointerSystem is currently responsible for the external call to the receiver's clear method: https://github.com/excaliburjs/Excalibur/blob/main/src/engine/Input/PointerSystem.ts#L163.

Expected Result

When removing the default PointerSystem, the engine or scene do not allocate or retain memory related to pointer events.

Actual Result

For every native pointer event:

  • Two event listener handlers are invoked (one for the engine and one for the scene).
  • Two references per pointer event are retained indefinitely (one by the by the engine's PointerEventReceiver and one by the scene's PointerEventReceiver).

Environment

  • browsers and versions: Chrome 131.0.6778.265
  • operating system: MacOS
  • Excalibur versions: 0.30.3
  • The memory leak does not depend on the environment.

Current Workaround

Detach both input receivers when initializing the scene that removes the default pointer system:

  public override onInitialize(): void {
    this.world.remove(this.world.get(PointerSystem));
    this.input.pointers.detach();
    this.engine.input.pointers.detach();
  }

Potential Quick Fix

Normally, the PointerSystem calls clear() on both input receivers at the end of its update loop. Alternatively (or additionally), clear() could be called by the pointer event receiver itself as the first step to its update().

Potential Robust Fix

The quick fix is not ideal because it still incurs superfluous allocation and execution. Rather than having the engine and scene manage pointer event listeners, the pointer system itself should manage the listeners. This way the system can remove the listeners when deactivated, disabled, disposed, and/or removed.

@eonarheim
Copy link
Member

@blakearoberts Thanks for the bug! Definitely an issue...

I do agree that perhaps pointer management should wholly be contained within the PointerSystem. It definitely feels right.

The current state is a vestige of Excalibur's evolution over time to an ECS based engine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants