Fix memory leak

[dylang]

graceful-fs is relied on by many major projects. Test frameworks, such as jest, both reset the module cache between tests to facilitate mocking, and rely on graceful-fs.

Because of this, graceful-fs is re-wrapping fs multiple times, causing ever-growing memory leaks. In a project with a a few thousand tests, this balloons to GB.

Three memory leaks were fixed in this PRz

• fs.close was getting re-wrapped every time graceful-fs was loaded.
• fs.closeSync was getting re-wrapped every time graceful-fs was loaded.
• The same clone of fs generated by ./fs.js was getting re-wrapped every time graceful-fs was loaded.

These memory leaks show up when:

• More than one version of graceful-fs is required.
• The module cache is cleared, which happens in Jest and other test frameworks.
• The impact of this leak is multiplied as thousands of modules rely on this package.

Note:
--enable-gc was added to the test runner so that we can monitor memory usage in one of the tests.

Fixes #102

Some related tickets:

• Memory leak in jest-circus jestjs/jest#7274
• [Memory Leak] on module loading jestjs/jest#6738
• Jest leaks memory from required modules with closures over imports jestjs/jest#6814
• Jest memory problems in node environment jestjs/jest#6399
• Ditch fs-extra mapbox/underreact#15
• We are leaking memory stylelint/stylelint#2168
• requiring library causes memory leak due to indirect dependency on node-graceful-fs forwardemail/email-templates#318

[tmax]

this is cool, but i have a noobish question. how do you test or "measure" the memory-leak-fix?

do you have a script to simulate typical usage and compare the expected memory usage vs actual?
any tools for doing that? interested to know how you'd solve this type of problem in general (tooling for fixing memory leaks).

[dylang]

Hi @tmax - great question!

If you look in test/avoid-memory-leak.js, you'll see we're using importFresh from the always amazing @sindresorhus to load the module.

Normally, node caches modules once they are required, but this library deletes the cache before loading so that we can reload it. This simulates what happens in frameworks like Jest that clear the module cache between tests.

First, we look at the memory usage with process.memoryUsage().heapUsed / Math.pow(1024, 2); The math is to convert it to Mb for easy reading.

Then we require "fresh" the module 4000 times, simulating 4000 unit tests.

We then call global.gc(); - this is an internal node command garbage collect. We normally don't need this, node will garbage collect as it has time, but in this
case we want to measure the memory impact right away.

Before this fix, garbage collecting had no effect because every copy of the module was still referenced by fs.

Finally, we look at the memory usage with process.memoryUsage().heapUsed / Math.pow(1024, 2) again, and compare to what we had before. A little growth is natural, so I made sure it's less than 2mb. Without the fix, the number can be in the hundreds of MB.

[isaacs]

Thanks! Landed on graceful-fs@4.1.12.

[gxcsoccer]

missing clone.js

[kirillgroshkov]

great work!

[dylang]

Thank you!

[addaleax]

@dylang This doesn’t seem to resolve the issue completely on Node 11 … do you think you could take a look? We generally run graceful-fs’s tests as part of each Node.js release, so having them pass is really nice for us :)

[addaleax]

ping @dylang ?

[DanielGibbsNZ]

This works fine on version 4.1.15 for fixing the memory leak, but when I upgrade to version 4.2.3 the memory leak comes back. Was a regression introduced with the changes in 4.2.x?

[dylanwulf]

I'm experiencing the same as @addaleax. I have this package locked at v4.1.15 but it's still causing memory leaks in node v11, v12, v13

[DanielGibbsNZ]

Using graceful-fs 4.2.4 on Node 12.19.1 seems to be working fine without any noticeable memory leak.

[manjaneqx]

🔌