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

Memory leak: animation proxy 'transitionend' event listener never removed #9046

Open
4 tasks done
samclaus opened this issue Jul 28, 2023 · 1 comment · May be fixed by #9048
Open
4 tasks done

Memory leak: animation proxy 'transitionend' event listener never removed #9046

samclaus opened this issue Jul 28, 2023 · 1 comment · May be fixed by #9048

Comments

@samclaus
Copy link

Checklist

  • I've looked at the documentation to make sure the behavior isn't documented and expected.
  • I'm sure this is an issue with Leaflet, not with my app or other dependencies (Angular, Cordova, React, etc.).
  • I've searched through the current issues to make sure this hasn't been reported yet.
  • I agree to follow the Code of Conduct that this project adheres to.

Steps to reproduce

Go to the constructor of Map and find:

DomEvent.on(this._proxy, 'transitionend', this._catchTransitionEnd, this);

There is no removal of the event listener in the teardown code for Map, which means that creating/tearing down maps repeatedly will leak memory over time.

Expected behavior

Map.remove() should contain the following:

DomEvent.off(this._proxy, 'transitionend', this._catchTransitionEnd, this);

Alternatively, it could be done in an 'unload' event listener.

Current behavior

Memory leak.

Minimal example reproducing the issue

No response

Environment

  • Leaflet version: 1.9.2
  • Browser (with version): any
  • OS/Platform (with version): M1 Max Macbook Pro
@samclaus samclaus added bug needs triage Triage pending labels Jul 28, 2023
@jonkoops
Copy link
Collaborator

Thanks for taking the time to report this with clear instructions, that is always appreciated. If you feel like you have an appropriate fix for this issue, a PR is always welcome.

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

Successfully merging a pull request may close this issue.

4 participants