VM's context when evaluating ESM script not garbage-collected (works with CJS) #50964

Activity

1. joyeecheung commented on Nov 30, 2023

   

   joyeecheung

   on Nov 30, 2023

   Last edited by joyeecheung

   Member

   More actions

   If you take heap snapshots before and after you set the ctx = null you can actually see that the context can be GC'ed.

   The leak detector is not implemented in a reliable way:

   1. The invocation of FinalizationRegistry callbacks is not an indication of whether an object can be garbage collected. See the explainer of the original TC39 proposal which explicitly warns about this https://github.com/tc39/proposal-weakrefs#another-note-of-caution - we also know from experience that when ephemeron GC is involved (e.g. when WeakMap is used, which is the case for vm script/modules) it's easy to produce false positives in leak detection that relies on the invocation of FinalizationRegistry callbacks.
   2. --expose-gc is not meant to "clean up the heap thoroughly", in fact it can actually prevent code from aging thus making certain scripts immortal. This is a V8 testing flag so what it does only serves V8's own use cases in tests, but not for general usage https://bugs.chromium.org/p/v8/issues/detail?id=12198#c4

   I see that the leak detector seems to come from an old implementation in Jest, I ran into a similar false positive in their previous implementation too, the workaround they did was generating a heap snapshot to "force" a GC: #48510 (comment) (I think this is still somewhat unreliable though, as both 1 and 2 are still true with that approach, it just lowers the chances of false positives but does not eliminate it, though in Node.js core we've still encountered false positives with a similar approach).
2. joyeecheung

   added

   invalidIssues and PRs that are invalid.Issues and PRs that are invalid.

   on Nov 30, 2023
3. joyeecheung

   closed this as completedon Nov 30, 2023
4. enisdenjo commented on Nov 30, 2023

   

   enisdenjo

   on Nov 30, 2023

   Author

   More actions

   @joyeecheung thank you for the thorough explanation! Much appreciated.
5. enisdenjo commented on Nov 30, 2023

   

   enisdenjo

   on Nov 30, 2023

   Author

   More actions

   @joyeecheung one more question if I may, why does the leak-detector work "reliably" for CJS scripts?
6. joyeecheung commented on Nov 30, 2023

   

   joyeecheung

   on Nov 30, 2023

   Member

   More actions

   It may just happen to “work” with the given graph (e.g. somehow this leads to enough pressure at the right time for the GC to run in a particular way to get the callback invoked) but it could very well stop working when the implementation of vm.Script or V8’s GC change in a subtle way, which is common for GC-related issues