nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
108.09k stars 29.83k forks source link

Possible memory leak with Proxy.revocable's revoker holding the proxy directly #37611

Closed ajvincent closed 3 years ago

ajvincent commented 3 years ago

What steps will reproduce the bug?

node --expose-gc revoke-leaks-proxy.js.txt

How often does it reproduce? Is there a required condition?

Reproduces 100% of the time

What is the expected behavior?

testGC Passed 0 testRevokeLeak Passed 0

What do you see instead?

testGC Passed 0 testRevokeLeak Failed 0

Additional information

I filed this at https://bugs.chromium.org/p/v8/issues/detail?id=11507#c2 as I believe it's really a V8 bug that the Node engine inherits. The V8 triage team required I file the bug here for Node engineers to determine if the bug is "with V8 or with their usage of V8?".

Reference: https://github.com/tc39/ecma262/issues/2325 SpiderMonkey bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1694781 ECMA-262 section: https://tc39.es/ecma262/#sec-proxy.revocable

Quick example code: const revokers = []; { const {revoke} = Proxy.revocable({}, Reflect); revokers.push(revoke); // the created proxy is inaccessible, but the revoker holds it in an internal slot }

The attached file uses FinalizationRegistry, Promise and node's global.gc() to see if a Proxy.revocable() call holds the proxy strongly. Based on documentation I've read online and the above spec issue (consensus is it's not a spec bug, but points to implementers for a fix), I believe this is a V8 bug which Node inherits.

Granted, it's in an obscure, rarely used part of ECMAScript...

devsnek commented 3 years ago

@ajvincent you can run this in d8, v8's debug cli to see if it happens without node. you can fetch the latest version of d8 using npmjs.org/esvu.

ajvincent commented 3 years ago

With a couple tweaks, I was able to run the same test in d8, and got the same result. So, do I go back to them and politely ask them to reopen the bug?

ajvincent commented 3 years ago

revoke-leaks-proxy-d8.js.txt

devsnek commented 3 years ago

I will reopen it.