Possible memory leak when removing raster sources & layers

[Scarysize]

mapbox-gl-js version: 0.39.1

Steps to Trigger Behavior

1. Open https://jsbin.com/moyotihaga/edit?html,output
2. Take a heap snapshot using your browser's memory tool
3. Hit the "Start" button. Over a period of 2 minutes 5 raster sources and layers are added and removed from the map in an 500ms interval.
4. After the button displays "Done!", take another heap snapshot

Expected Behavior

Memory usage is only slightly higher compared to the initial snapshot.

Actual Behavior

Memory usage stays higher even though sources and layers have been removed from the map.

---

Here is a standalone example, so this can be tested without the overhead of JSBIN (it's a single index.html file):
mapbox-memory-test.html.zip

To give you a better impression why we are adding & removing a huge amount of layers & sources:

We got a time slider component which updates which layer is displayed on the map. Basically every black bar in the slider represents a raster source & a raster layer. The layer and its source are added and removed from the map depending on the slider position. You can see the black bar increasing in size, that's when they are added to the map.

In the GIF you can see the application with only two activated overlay types. The application offers a selection >50 different raster overlay types. Also the application is very long running (~a business day) without being refreshed/reloaded. So during its "lifetime" a lot of layers and sources are removed, leading to ever increasing memory usage.

---

We use multiple layers and sources because there isn't really a equivalent to the setData() method of geojson sources. We would need something like setTileUrl() on raster sources, so we don't have to add and remove a bunch of new sources and layers all the time. Is there a possibility of this being implemented? What are the concerns? I think with some hand holding, I could try to contribute this with PR; but it's probably not a minor thing to add.

We already fixed a similar issue ( #3951)

---

• Chrome Version 60.0.3112.101 (Official Build) (64-bit)
• macOS 10.12.6 (MBP mid-2015)

[anandthakker]

Thanks for the report & diagnostics, @Scarysize!

We would need something like setTileUrl() on raster sources, so we don't have to add and remove a bunch of new sources and layers all the time. Is there a possibility of this being implemented?

In the next few weeks, we're going to start working on the "custom sources" project -- I think there's a good chance that a addressing this will be more straightforward in that context.

[Scarysize]

Correct me if I'm wrong, but with custom sources we probably lose a lot of stuff which is build into mapbox raster sources now. E.g. everything concerning tiling (requesting correct child and parent tiles possible in regard to some specified bounds), caching and more.

Finding and fixing this memory leak/usage probably isn't a high priority in this project. It's a pain to investigate and probably impossible without intricate knowledge of the code base. Also we probably got a very unusual use case here, otherwise more people would have complained by now.

Are there any quick solutions we can go for? Any advice? I will probably do some more digging, maybe I can find some obvious leaks.

[anandthakker]

Correct me if I'm wrong, but with custom sources we probably lose a lot of stuff which is build into mapbox raster sources now. E.g. everything concerning tiling (requesting correct child and parent tiles possible in regard to some specified bounds), caching and more

In fact, much of this -- calculating visible tiles, caching tiles, using parent tiles until child tiles are finished loading, etc.

Are there any quick solutions we can go for? Any advice? I will probably do some more digging, maybe I can find some obvious leaks

Nothing immediately obvious comes to mind, although I do have one question: in which browser(s) are you seeing the leak? #4695 describes a safari-specific leak, and I'm wondering if that's related.

[Scarysize]

Nothing immediately obvious comes to mind, although I do have one question: in which browser(s) are you seeing the leak? #4695 describes a safari-specific leak, and I'm wondering if that's related.

• Chrome Version 60.0.3112.101 (Official Build) (64-bit)
• macOS 10.12.6 (MBP mid-2015)

(also added these to the initial bug report up top ^)

[Scarysize]

@anandthakker I found something interesting by accident:

When map.removeSource is called for a raster source, the RasterTileSource instance is still retained/not gcc. If I read the heap snapshot correctly the serialize() & load() methods of the sources somehow keep the instance alive.

Here is a jsfiddle to reproduce this: https://jsfiddle.net/4km3mga7/1/
(The fiddle uses the mapbox dev build in order to keep class names unobfuscated.)

1. Open fiddle, wait for map to load
2. Make a heap snapshot
3. Search the snapshot for instances of "RasterTileSource" (there is a class filter input field in Chrome)
4. Hit the "Remove Source" Button
5. Repeat step 3, the "RasterTileSource" instance is still in memory

The instance has a "distance" to the root object of "-", so it seems to be unreachable by GC.

I hope this gives a clue to what might be the problem.

[jfirebaugh]

@Scarysize It doesn't reproduce for me. The second snapshot doesn't have any matches for a "RasterTileSource" search.

[Scarysize]

Weird. I will check if I can reproduce this on another machine. In the meantime I reproduced this in a incognito Chrome tab too.

[Scarysize]

Nice!