-
Notifications
You must be signed in to change notification settings - Fork 224
Description
Description
We got reports of memory leaks from users of React InstantSearch with Next.js.
It's easy to reproduce, with an index.mjs
file :
import process from 'process';
import { algoliasearch } from 'algoliasearch';
process.stdin.resume();
const client = algoliasearch('latency', '6be0576ff61c053d5f9a3225e2a90f76');
for (let i = 0; i < 1000; i++) {
client.search({ requests: [{ indexName: 'instant_search' }] });
}
Then run node --inspect index.mjs
, in Chrome open chrome://inspect
and select index.mjs
.
In the memory tab, the VM instance heap size hovers around 90MB without ever going down.

All responses are retained by the error handler.
I tried patching it by adding req.removeAllListeners()
as well as response.removeAllListeners()
in this end
handler :
algoliasearch-client-javascript/packages/requester-node-http/src/createHttpRequester.ts
Lines 60 to 69 in 36ccbc9
response.on('end', () => { | |
clearTimeout(connectTimeout as NodeJS.Timeout); | |
clearTimeout(responseTimeout as NodeJS.Timeout); | |
resolve({ | |
status: response.statusCode || 0, | |
content: Buffer.concat(contentBuffers).toString(), | |
isTimedOut: false, | |
}); | |
}); |
It's not perfect but it now settles around 20MB. Some unnecessary values are still there but maybe this will properly get GC'd if needed.
I told them to use requester-fetch
instead as it's now supported by recent versions of Node.js. Maybe we could make that the default ? From my own benchmarks it seems better than the Node requester currently but with my proposed fix it would make the Node requester more performant than the fetch one 😅 (with undici)
Client
All
Version
5.15.0
Relevant log output
No response