MapiResponse never gets garbage collected in Node

[markgandy]

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)

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?

[markgandy]

Just following up on this. We ended up removing the mapbox SDK and calling the directions REST API directly instead. You can see pretty clearly in the graph, I think it's pretty obvious when we made the change! Heap would just keep growing until eventually out of memory. So looks to definitely be an issue.

[kascakm]

I'm having the same issue. Looking through the code it seems that this object here requestsUnderway tracks all requests but would remove them only if the request is aborted. https://github.com/mapbox/mapbox-sdk-js/blob/main/lib/node/node-layer.js#L15

[tetchel]

This seems like a significant issue

@mpothier @cmtoomey

Sorry for the tags but I would like to see a maintainer response on this one before adopting this SDK.

[mikksoone]

We got hit by this as well. Thanks @kascakm for the pointer to the leak - this helped us verify it was the same problem. We also moved to web api and now everything is fine.

[tonda100]

We also have to move rest api because of this issue.

[mehmetmalli]

I created a PR for this: https://github.com/mapbox/mapbox-sdk-js/pull/496

tested locally, seems to fix the memory build up issue