mock.restore() leaks (no garbage collection)

[nonara]

Bug Report

Hello,

Unfortunately, it seems that garbage collection is not working after mock.restore().

In my case, I have a collection of files which are pre-loaded. The total size is around 100mb.

The format is:

{
  "/path/dir": {},
  "/path/dir/file.ext": "file content"
}

When I call mock.restore(), the expectation is that the memory usage would go down, however it does not.

Re-initializing mock() with the same object causes the memory to go up. Just a few calls to mock/restore puts me over 1.4gb very quickly. I discovered this because GH actions started failing due to exceeding oldspace size for Node 10.

Workaround

If anyone encounters the same issue, here is my workaround, which uses file caching that we can control

import { normalizeSlashes } from 'ts-node';
import shell from 'shelljs';
import path from 'path';
import fs from 'fs';
import FileSystem from 'mock-fs/lib/filesystem';
import { default as mock } from 'mock-fs';


/* ***************************************************** */
// Constants
/* ***************************************************** */
const cachedFiles = new Map<string, Buffer>();
const vanillaFiles = cacheDirectories(
  '/path/to/files', 
  '/path/to/otherfiles'
);


/* ***************************************************** */
// Helpers
/* ***************************************************** */
/**
 * Create a DirectoryItems from actual files and directories for mock-fs (uses caching)
 */
function cacheDirectories(...directory: string[]): FileSystem.DirectoryItems {
  const res: FileSystem.DirectoryItems = {};
  for (const dir of directory)
    for (const stat of shell.ls('-RAl', dir) as unknown as (fs.Stats & { name: string })[]) {
      const statPath = normalizeSlashes(path.join(dir, stat.name));
      res[statPath] =
        !stat.isFile() ? {} :
        <ReturnType<typeof mock.file>>(() => {
          const fileData = fs.readFileSync(statPath);
          cachedFiles.set(statPath, fileData);

          return Object.defineProperty(mock.file({ content: '' })(), '_content', {
            get: () => cachedFiles.get(statPath),
            set: (data: any) => cachedFiles.set(statPath, data)
          });
        });
    }

  return res;
}


/* ***************************************************** */
// Exports
/* ***************************************************** */
export function mockFs() {
  mock(vanillaFiles);
}

export function restoreFs() {
  mock.restore();
  cachedFiles.clear();
}

Feature Request

Access to original fs

It would be wonderful to have the ability to access the original fs module. Originally, I wanted to do a lazy load solution, which allowed me to only cache files when a readFileSync was called.

I'm mocking full node projects with node_modules (testing for https://npmjs.com/ts-patch), but many files never get loaded. It would be great to not have to load them.

It's easy to integrate with the above logic, however however, we'd need the ability to read file data from the actual filesystem. The only way I found to do that was by child_prcoess.spawnSync to create a new node instance and get the data from the output buffer. This was unfortunately too slow.

TLDR; Would love the have an fsOriginal accessible, or, minimally, readFileSync, so we could lazy-load and cache

[3cp]

I think I created that leak.

mock-fs/lib/binding.js

Lines 243 to 266 in e42fdbb

	// I don't know exactly what is going on.
	// If _openFiles is a property of binding instance, there is a strange
	// bug in nodejs v10+ that something cleaned up this._openFiles from
	// nowhere. It happens after second mockfs(), after first mockfs()+restore().
	// So I moved _openFiles to a private var. The other two vars (_system,
	// _counter) do not hurt.
	// This fixed https://github.com/tschaub/mock-fs/issues/254
	// But I did not dig deep enough to understand what exactly happened.
	let _system;
	let _openFiles = {};
	let _counter = 0;
	/**
	* Create a new binding with the given file system.
	* @param {FileSystem} system Mock file system.
	* @constructor
	*/
	function Binding(system) {
	/**
	* Mock file system.
	* @type {FileSystem}
	*/
	_system = system;

It was to bypass some weird behaviour on nodejs v10+ when I did not understand what's going on.
Then I vaguely remember that I figured it out in following work on other fixes. But I now forgot the details again 😅, this hack probably should be removed.

I will try to reclaim my lost memory, and provide you a build to test.

[nonara]

I think I created that leak.

Haha. No worries. I've been running into some extremely bizarre behaviour dealing with node / v8 lately, so I completely understand.

Since you're familiar with the architecture, what are you thoughts on the feasibility of providing a route toward reading a file outside of the mocked system?

If such a function was available, I'd be glad to do a PR, re-engineering my logic into a helper that allowed people to populate a FileSystem.DirectoryItems object from a set of real paths with the option to pre-load or lazyload files.

[3cp]

The similar feature was requested before, but nobody picked it up yet. #62, #213

[3cp]

Can you test locally against npm install tschaub/mock-fs#fix-leak?

[nonara]

@3cp Cool! Will have a look at it tomorrow. Meanwhile, I just wrapped up this pull request, which I think should help ease everyone's frustrations without doing any major changes to the core.

Let me know what you think.