Skip to content

Commit

Permalink
Fix memory leak in Worker when map is removed (#3733) (#3734)
Browse files Browse the repository at this point in the history
* Remove data inside Worker when map is remove (#3733)

* Add tests

* Update changelog

* Simplify test assertion
  • Loading branch information
pasieronen authored Feb 27, 2024
1 parent e0daa5d commit 2095380
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### 🐞 Bug fixes
- Fix popup appearing far from marker that was moved to a side globe ([3712](https://github.com/maplibre/maplibre-gl-js/pull/3712))
- Set text color to ensure contrast in the attribution pill ([3737](https://github.com/maplibre/maplibre-gl-js/pull/3737))
- Fix memory leak in Worker when map is removed ([3734](https://github.com/maplibre/maplibre-gl-js/pull/3734))
- _...Add new stuff here..._

## 4.0.2
Expand Down
7 changes: 7 additions & 0 deletions src/source/worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,11 @@ describe('Worker register RTLTextPlugin', () => {
worker.actor.messageHandlers['setImages']('0', ['availableImages']);
expect(worker.availableImages['0']).toEqual(['availableImages']);
});

test('clears resources when map is removed', () => {
worker.actor.messageHandlers['setLayers']('0', []);
expect(worker.layerIndexes['0']).toBeDefined();
worker.actor.messageHandlers['removeMap']('0', undefined);
expect(worker.layerIndexes['0']).toBeUndefined();
});
});
7 changes: 7 additions & 0 deletions src/source/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,13 @@ export default class Worker {
}
});

this.actor.registerMessageHandler('removeMap', async (mapId: string) => {
delete this.layerIndexes[mapId];
delete this.availableImages[mapId];
delete this.workerSources[mapId];
delete this.demWorkerSources[mapId];
});

this.actor.registerMessageHandler('setReferrer', async (_mapId: string, params: string) => {
this.referrer = params;
});
Expand Down
3 changes: 3 additions & 0 deletions src/style/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1493,6 +1493,9 @@ export class Style extends Evented {
}
this.imageManager.setEventedParent(null);
this.setEventedParent(null);
if (mapRemoved) {
this.dispatcher.broadcast('removeMap', undefined);
}
this.dispatcher.remove(mapRemoved);
}

Expand Down
7 changes: 7 additions & 0 deletions src/ui/map.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,13 @@ describe('Map', () => {
canvas.dispatchEvent(new window.Event('webglcontextlost'));
});

test('#remove broadcasts removeMap to worker', () => {
const map = createMap();
const _broadcastSpyOn = jest.spyOn(map.style.dispatcher, 'broadcast');
map.remove();
expect(_broadcastSpyOn).toHaveBeenCalledWith('removeMap', undefined);
});

test('#redraw', async () => {
const map = createMap();

Expand Down
1 change: 1 addition & 0 deletions src/util/actor_messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export type RequestResponseMessageMap = {
'syncRTLPluginState': [PluginState, boolean];
'setReferrer': [string, void];
'removeSource': [RemoveSourceParams, void];
'removeMap': [undefined, void];
'importScript': [string, void];
'removeTile': [TileParameters, void];
'abortTile': [TileParameters, void];
Expand Down

0 comments on commit 2095380

Please sign in to comment.