Skip to content

Commit

Permalink
fix(mocha-runner): fix memory leaks
Browse files Browse the repository at this point in the history
Fix a memory leak that keeps required files in-memory between test runs. Properly clear the require cache by also clearing [the mocha's module children](https://nodejs.org/api/modules.html#modules_module_children). This makes sure the required files by mocha can be garbage collected.
  • Loading branch information
nicojs committed Sep 9, 2020
1 parent e90b563 commit 23eede2
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/mocha-runner/src/MochaTestRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class MochaTestRunner implements TestRunner2 {
public async init(): Promise<void> {
this.mochaOptions = this.loader.load(this.options as MochaRunnerWithStrykerOptions);
this.testFileNames = this.mochaAdapter.collectFiles(this.mochaOptions);
this.requireCache.init(this.testFileNames);
this.requireCache.init({ initFiles: this.testFileNames, rootModuleId: require.resolve('mocha/lib/mocha') });
if (this.mochaOptions.require) {
this.rootHooks = await this.mochaAdapter.handleRequires(this.mochaOptions.require);
}
Expand Down
18 changes: 18 additions & 0 deletions packages/mocha-runner/test/integration/MemoryLeak.it.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import path = require('path');

import execa = require('execa');

import { expect } from 'chai';

import { MochaTestRunner } from '../../src';

describe(MochaTestRunner.name, () => {
it('should not leak memory when running multiple times (#2461)', async () => {
const childProcess = await execa.node(path.resolve(__dirname, 'MemoryLeak.worker.js'), [], {
stdio: 'pipe',
nodeOptions: ['--max-old-space-size=32', '--max-semi-space-size=1'],
});
expect(childProcess.exitCode).eq(0);
expect(childProcess.stdout).contains('Iterator count 1');
});
});
45 changes: 45 additions & 0 deletions packages/mocha-runner/test/integration/MemoryLeak.worker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import path = require('path');

import { testInjector } from '@stryker-mutator/test-helpers';

import { expect } from 'chai';
import { DryRunStatus, TestStatus } from '@stryker-mutator/api/test_runner';

import { createMochaTestRunner } from '../../src';

/**
* This file will run the mocha runner a number of times in a test suite that is designed
* to result in an OutOfMemory error when the mocha test runner does not clean it's memory
* Start this process with `--max-old-space-size=32 --max-semi-space-size=1` to get a fast OutOfMemory error (if there is a memory leak)
*
* @see https://github.com/stryker-mutator/stryker/issues/2461
* @see https://nodejs.org/api/modules.html#modules_accessing_the_main_module
* @see https://stackoverflow.com/questions/30252905/nodejs-decrease-v8-garbage-collector-memory-usage
*/

if (require.main === module) {
main().catch((err) => {
console.error(err);
process.exitCode = 1;
});
}

async function main() {
process.chdir(path.resolve(__dirname, '..', '..', 'testResources', 'big-project'));
const mochaRunner = testInjector.injector.injectFunction(createMochaTestRunner);

await mochaRunner.init();
await doDryRun();

async function doDryRun(n = 20) {
if (n > 0) {
console.log(`Iterator count ${n}`);
const result = await mochaRunner.dryRun({ coverageAnalysis: 'off', timeout: 3000 });
if (result.status === DryRunStatus.Complete) {
expect(result.tests).lengthOf(1);
expect(result.tests[0].status).eq(TestStatus.Success);
}
await doDryRun(n - 1);
}
}
}
5 changes: 4 additions & 1 deletion packages/mocha-runner/test/unit/MochaTestRunner.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ describe(MochaTestRunner.name, () => {

await sut.init();

expect(directoryRequireCacheMock.init).calledWithExactly(expectedTestFileNames);
expect(directoryRequireCacheMock.init).calledWithExactly({
initFiles: expectedTestFileNames,
rootModuleId: require.resolve('mocha/lib/mocha'),
});
});

it('should not handle requires when there are no `requires`', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

// Change the text each iteration to prevent nodejs from optimizing it into one value on the heap
if (!global.n) {
global.n = 500000;
}
module.exports.text = new Array(global.n++).fill('i').join(',');
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const { text } = require('../src/big-text');

describe('Big text', () => {
it('should contain i', () => {
if (text.indexOf('i') === -1) {
throw new Error();
}
});
});
15 changes: 14 additions & 1 deletion packages/util/src/DirectoryRequireCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@ import path = require('path');
* A helper class that can be used by test runners.
* The first time you call `record`, it will fill the internal registry with the files required in the current working directory (excluding node_modules)
* Then each time you call `clear` it will clear those files from the require cache
*
* It will also delete the `module.children` property of the root module.
* @see https://github.com/stryker-mutator/stryker/issues/2461
*/
export class DirectoryRequireCache {
private cache: Set<string>;
private initFiles?: readonly string[];
private rootModuleId: string;

public init(initFiles: readonly string[]) {
public init({ initFiles, rootModuleId }: { initFiles: readonly string[]; rootModuleId: string }) {
this.initFiles = initFiles;
this.rootModuleId = rootModuleId;
}

/**
Expand All @@ -29,6 +34,14 @@ export class DirectoryRequireCache {

public clear() {
if (this.cache) {
if (this.rootModuleId) {
const rootModule = require.cache[this.rootModuleId];
if (rootModule) {
rootModule.children = rootModule.children.filter((childModule) => !this.cache.has(childModule.id));
} else {
throw new Error(`Could not find "${this.rootModuleId}" in require cache.`);
}
}
this.cache.forEach((fileName) => delete require.cache[fileName]);
}
}
Expand Down
41 changes: 38 additions & 3 deletions packages/util/test/unit/DirectoryRequireCache.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@ describe(DirectoryRequireCache.name, () => {
let workingDirectory: string;
let sut: DirectoryRequireCache;
let loadedFiles: Map<string, string>;
let rootModule: NodeModule;

beforeEach(() => {
loadedFiles = new Map();
workingDirectory = path.join('stub', 'working', 'dir');
const cwdStub = sinon.stub(process, 'cwd').returns(workingDirectory);
cwdStub.returns(workingDirectory);
sut = new DirectoryRequireCache();
rootModule = createModule('root', 'root');
require.cache['root'] = rootModule;
});

afterEach(() => {
Expand All @@ -25,9 +28,11 @@ describe(DirectoryRequireCache.name, () => {
}
});

function fakeRequireFile(fileName: string, content = fileName) {
function fakeRequireFile(fileName: string, content = fileName, requiredBy = rootModule) {
loadedFiles.set(fileName, content);
require.cache[fileName] = createModule(content, fileName);
const child = createModule(content, fileName);
require.cache[fileName] = child;
requiredBy.children.push(child);
}

describe(DirectoryRequireCache.prototype.clear.name, () => {
Expand All @@ -40,7 +45,7 @@ describe(DirectoryRequireCache.name, () => {
fakeRequireFile(fooFileName, 'foo');
fakeRequireFile(barFileName, 'foo');
fakeRequireFile(bazFileName, 'baz');
sut.init([fooFileName, barFileName]);
sut.init({ initFiles: [fooFileName, barFileName], rootModuleId: 'root' });
sut.record();

// Act
Expand Down Expand Up @@ -72,6 +77,36 @@ describe(DirectoryRequireCache.name, () => {
expect(require.cache[bazFileName]?.exports).eq('baz');
});

it('should clear recorded children from the root', () => {
// Arrange
const dir2 = path.join('stub', 'working', 'dir2');
const fooFileName = path.join(workingDirectory, 'foo.js');
const barFileName = path.join(workingDirectory, 'bar.js');
const bazFileName = path.join(dir2, 'baz.js');
fakeRequireFile(fooFileName, 'foo');
fakeRequireFile(barFileName, 'foo');
fakeRequireFile(bazFileName, 'baz');
expect(rootModule.children).lengthOf(3);
sut.init({ initFiles: [], rootModuleId: 'root' });
sut.record();

// Act
sut.clear();

// Assert
expect(rootModule.children).lengthOf(1);
expect(rootModule.children[0].filename).eq(bazFileName);
});

it("should throw when the root module wasn't loaded", () => {
// Arrange
sut.init({ initFiles: [], rootModuleId: 'not-exists' });
sut.record();

// Act
expect(() => sut.clear()).throws('Could not find "not-exists" in require cache.');
});

it('should not clear files from node_modules', () => {
// Arrange
const fooFileName = path.join(workingDirectory, 'foo.js');
Expand Down

0 comments on commit 23eede2

Please sign in to comment.