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

Overview map causes memory leak #6377

Closed
vsa opened this issue Jan 17, 2017 · 5 comments
Closed

Overview map causes memory leak #6377

vsa opened this issue Jan 17, 2017 · 5 comments

Comments

@vsa
Copy link

vsa commented Jan 17, 2017

Removing a map (with an overview map control) from the document leaks memory. Here is a fiddle for reproducing the issue: http://jsfiddle.net/pefn7L5n/2/.

This looks similar to #3420 (handleResize_ appears in the memory profiler), but is related to the overview map control (when no such control is added to the map the leak no longer occurs).

Is there some other way to explicitly destroy the map (other then call map.setTarget(null))?

@ahocevar
Copy link
Member

ahocevar commented Jan 17, 2017

In your fiddle, you loop through all controls and remove them from the map. Does the situation change if you don't do that?

map.setTarget(null) should be enough to make the map a candidate for garbage collection. Maybe you took a memory profile before it was garbage collected? Most browsers' developer consoles have an option to manually trigger garbage collection.

@vsa
Copy link
Author

vsa commented Jan 17, 2017

No, throwing out everything except map.setTarget(null) doesn't help, see http://jsfiddle.net/pefn7L5n/3/. I only added the rest because I was hoping to have overlooked something else that requires cleanup (thank you for the clarification).

I'm aware of the garbage collection, but that's not the issue at hand. Do you need a more detailed explanation on how I profiled this?

@ahocevar
Copy link
Member

Ok, so the issue here is probably that the overview map's target is never set to null. You could manually do that with overviewMapControl.getOverviewMap().setTarget('null'), but we'd also accept a pull request that does this in ol.control.OverviewMap#setMap.

@vsa
Copy link
Author

vsa commented Jan 18, 2017

That helps, thank you for the suggestion.

Unfortunately I won't be able to provide a pull request anytime soon because my current environment does not support make.

@fredj
Copy link
Member

fredj commented Jun 21, 2018

fixed with #6379

@fredj fredj closed this as completed Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants