mapbox / mapbox/mapbox-sdk-js

MapiResponse never gets garbage collected in Node

Open
#396 7 comments 5 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
768
Forks
191
PR merge metrics
No merged PRs in 30d

Description

We have a memory leak issue with a Node app that appears to be caused by MapiResponse never getting cleaned up by the garbage collecter. We are creating a client and using to get directions as below using Node v12.16:
```
const mapboxDirectionsFactory = require('@mapbox/mapbox-sdk/services/directions');

class MapboxAdapter {
constructor(options) {
this.client = mapboxDirectionsFactory(options);
}
directionsConsideringTraffic({geoLocations}) {
return this.client.getDirections({
profile: 'driving-traffic',
geometries: 'geojson',
steps: true,
waypoints: geoLocations.map(coordinates => ({coordinates}))
}).send();
}
}

module.exports = {
MapboxAdapter
};
```

But we can see that `MapiRequest` and `MapiResponse` never get cleaned up. Looking at heap snapshots from prod we can see these just keep growing (example here has grown up to 90MB just for `MapiResponse`)
![image](https://user-images.githubusercontent.com/3522980/92557319-48a20e00-f2af-11ea-9024-a956a70aeec4.png)

I have also reproduced running locally in the Chrome debugger. I can see these objects build up and never get removed even when I manually run a GC in Chrome.

It looks like the SDK uses the global namespace so I guess this is why they are never GC'd? Is there something special we should be doing in Node to avoid this issue?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the directions call in the issue's MapboxAdapter example with Node v12.16, then inspect heap snapshots and forced-GC behavior for retained MapiRequest and MapiResponse objects. Done means identifying the retention path and confirming that repeated requests no longer cause these objects to grow indefinitely.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.