Skip to content

Commit

Permalink
Fix for memory leak caused by referenced to deferred test functions
Browse files Browse the repository at this point in the history
(it/before[Each]/after[Each]).

related to #1991.
  • Loading branch information
bd82 committed Jan 5, 2016
1 parent 5722c2b commit 33c825b
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 0 deletions.
48 changes: 48 additions & 0 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var stackFilter = utils.stackTraceFilter();
var stringify = utils.stringify;
var type = utils.type;
var undefinedError = utils.undefinedError;
var isArray = utils.isArray;

/**
* Non-enumerable globals.
Expand Down Expand Up @@ -719,6 +720,49 @@ Runner.prototype.uncaught = function(err) {
this.emit('end');
};

/**
* Cleans up the reference to the test's deferred function.
* @see cleanSuiteReferences for details.
* @param {Test} test
*/
function cleanTestReferences(test) {
delete test.fn;
}

/**
* Cleans up the references to all the deferred functions
* (before/after/beforeEach/afterEach) of a Suite.
* These must be deleted otherwise a memory leak can happen,
* as those functions may reference variables from closures,
* thus those variables can never be garbage collected as long
* as the deferred functions exist.
*
* @param {Suite} suite
*/
function cleanSuiteReferences(suite) {
function cleanArrReferences(arr) {
for (var i = 0; i < arr.length; i++) {
delete arr[i].fn;
}
}

if (isArray(suite._beforeAll)) {
cleanArrReferences(suite._beforeAll);
}

if (isArray(suite._beforeEach)) {
cleanArrReferences(suite._beforeEach);
}

if (isArray(suite._afterAll)) {
cleanArrReferences(suite._afterAll);
}

if (isArray(suite._afterEach)) {
cleanArrReferences(suite._afterEach);
}
}

/**
* Run the root suite and invoke `fn(failures)`
* on completion.
Expand Down Expand Up @@ -750,6 +794,10 @@ Runner.prototype.run = function(fn) {

debug('start');

// references cleanup to avoid memory leaks
this.on('test end', cleanTestReferences);
this.on('suite end', cleanSuiteReferences);

// callback
this.on('end', function() {
debug('end');
Expand Down
2 changes: 2 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ var isArray = typeof Array.isArray === 'function' ? Array.isArray : function(obj
return Object.prototype.toString.call(obj) === '[object Array]';
};

exports.isArray = isArray;

/**
* Buffer.prototype.toJSON polyfill.
*
Expand Down
47 changes: 47 additions & 0 deletions test/integration/fixtures/regression/issue-1991.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
function MemoryLeak() {
this.myArr = [];
for (var i = 0; i < 1000000; i++) {
this.myArr.push(i)
}
}

var numOfTests = 300;
for (var i = 0; i < numOfTests; i += 1) {
/*
* This Test suite will crash V8 due to:
* 'FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - process out of memory'
* if all the deferred functions references have not been cleared
*/
describe('Memory Leak Suite #' + i, function () {

// The <closureVar> variable will be accessed by the test below.
// As long as those test's functions are
// referenced in memory, the closure variable may not be garbage collected
// as it is still referenced.
// * In a chrome heap snapshot it will appear under "system / Context" (a scope)
var closureVar;

before(function () {
var x = closureVar ? 1 : 2
});

after(function () {
var x = closureVar[0]
});

beforeEach(function () {
var x = closureVar ? 1 : 2
});

afterEach(function () {
var x = closureVar[0]
});

it('access a variable via a closure', function () {
// slow performance on older node.js versions
this.timeout(1000);
closureVar = new MemoryLeak();
});

});
}
11 changes: 11 additions & 0 deletions test/integration/regression.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,15 @@ describe('regressions', function() {
done();
});
});

it('issue-1991: Declarations do not get cleaned up unless you set them to `null` - Memory Leak', function(done) {
// on a modern MBP takes ±5 seconds on node 4.0, but on older laptops with node 0.12 ±40 seconds.
// Could easily take longer on even weaker machines (Travis-CI containers for example).
this.timeout(120000);
run('regression/issue-1991.js', [], function(err, res) {
assert.equal(/process out of memory/.test(res.output), false, 'fixture\'s process out of memory!');
assert.equal(res.code, 0, 'Runnable fn (it/before[Each]/after[Each]) references should be deleted to avoid memory leaks');
done();
});
})
});

0 comments on commit 33c825b

Please sign in to comment.