-
-
Notifications
You must be signed in to change notification settings - Fork 536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MockHttpSocket
and MockHttpAgent
are not being garbage collected
#2405
Comments
This is causing a large memory leak meaning we have to restart our production pods every day. Any update / acknowledgement of the issue would be great. |
@kettanaito sorry for the direct ping and we really appreciate your work here, but this is causing us at Docker significant issues as use MSW in production to host a demo environment that matches our e2e tests and we have to restart production pods constantly due to this MSW memory leak. We can potentially contribute engineering effort if you can help point us in the right direction as it would be a large lift to migrate away from MSW for this. |
@mikeparker, I am sorry to hear that this is blocking you. If you have time, could you please see what in My initial guess is that it can be request or response parser, although we try to free it at the first available moment (1, 2). I will try to look into this but I am overburdened as of now. I would be grateful if you landed me a hand investigating this so I can roll out the appropriate fix as soon as we know what's causing the issue. Thanks. |
Another potential problem area is that the agent/socket interface may not have some of the methods implemented that result in it not being disposed of correctly. This can be proven/disproven by constructing and observing an actual |
I think I'm seeing something related: It would be nice to be able to use msw without the websocket interceptors being applied. |
@85B622ACA6, I don't believe we interfere with request at that level to result in the unexpected |
MockHttpSocket
and MockHttpAgent
are not being garbage collected
I believe you are correct. After playing around with it some more, the results I was seeing seem to be related to how msw interacts with the 'ws' WebSocket client. I hate to go of topic, but is msw expected to play nicely with clients from the 'ws' package? |
I've opened a pull request to try to measure the memory footprint of the interceptors in mswjs/interceptors#702. While I cannot see quite the same memory heap given more-or-less the same reproduction scenario, the memory snapshot with interceptors is ~50MB larger for 10,000 requests. Alas, I have no idea how to navigate the heap snapshot to understand why |
Yes, I believe so. We are using |
For node versions <22 too? (22 introduced the built-in WebSocket client) |
@85B622ACA6, yes, as long as |
That's a great idea, lemme poke around at that and see what pops up |
Ok so playing around with import net from 'node:net';
const handler = {
get(t, p, r) {
const v = Reflect.get(t, p, r);
console.log(`-----------------------------------------------------`);
console.log(
`get socket[${p.toString()}]:\n${
typeof v === 'function' ? 'function' : v
}`
);
if (typeof v === 'function') {
// trap for arguments
return new Proxy(v, {
apply(target, thisArg, args) {
console.log(`-----------------------------------------------------`);
console.log(`-> calling socket[${p.toString()}](${args.join(', ')})`);
return Reflect.apply(target, thisArg, args);
},
});
}
return v;
},
};
const server = net.createServer((c) => {
const s = new Proxy(c, handler);
s.pipe(s);
});
server.on('error', (err) => {
throw err;
});
server.listen(8124, () => {
console.log('server bound');
}); Kicking that up and then sending a 'foo' string into it with: $ echo "foo" | nc localhost 8124 Gives the following output. I'll still dig in some more, but maybe you can see something I don't 😅
|
@rossipedia it would be nice to compare that output to the same operation and proxy applied when using MSW. You can put it in a text diff online, that's how I usually A/B unclear issues like this. Really appreciate your time looking into it! I think we're on the right track comparing the handler. |
Argh... so the problem here is that when I try to wrap an outgoing socket in a Proxy, I get internal assertion errors. I'm admittedly a bit out of practice when it comes to this kind of low-level interaction with import net from 'node:net';
import http from 'node:http';
const req = http.get('http://localhost:8080', {
// createConnection: (options) => new Proxy(net.createConnection(options), {}),
});
req.on('response', (res) => {
res.resume();
res.on('end', () => {
console.log('Response received');
process.exit(0);
});
}); Results in: ❯ node socktest.mjs
node:internal/assert:11
throw new ERR_INTERNAL_ASSERTION(message);
^
Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
Please open an issue with this stack trace at https://github.com/nodejs/node/issues
at assert (node:internal/assert:11:11)
at Socket.socketOnData (node:_http_client:556:3)
at Socket.emit (node:events:524:28)
at addChunk (node:internal/streams/readable:561:12)
at readableAddChunkPushByteMode (node:internal/streams/readable:512:3)
at Readable.push (node:internal/streams/readable:392:5)
at TCP.onStreamRead (node:internal/stream_base_commons:189:23) {
code: 'ERR_INTERNAL_ASSERTION'
}
Node.js v22.13.0 Which points at this assert. I'm wondering if the Proxy is not getting added early enough. |
Prerequisites
Environment check
msw
versionNode.js version
22.12.0
Reproduction repository
https://github.com/rossipedia/msw-leak-test
Reproduction steps
Instructions are in the reproduction repo, but they are essentially:
npm install
./demo.sh
Current behavior
Note
This is a follow up to my colleague's issue #2390
Enabling MSW with no handlers and then load-testing using
ab
causes heap usage to balloon, even when manually invoking garbage collection.As best I can tell, the largest percentage of retained objects are primarily:
MockHttpSocket
MockHttpAgent
I do not believe this is related to
express
, as onlyClientRequest
is intercepted, and I don't thinkexpress
orhttp.createServer
useClientRequest
, onlyIncomingMessage
andServerResponse
(I could be wrong about that though).Expected behavior
Memory usage should be stable under load when simply enabling MSW in a node application.
The text was updated successfully, but these errors were encountered: