Memory leaks because of closures

[saskodh]

Hi, this is the first issue that I am creating after almost 2 years of usage of your extraordinary testing framework for JavaScript. I am using it mainly for testing AngularJs code and I love it, it enables me to test every peace of my code and with the BDD syntax every test simply becomes part of my code documentation.

Recently after some extensive debugging, reading and analysing your source code I finally found out why I was experiencing memory leak problems. The problem was that the running tests were not able to finish because of the build up memory consumption in the browser caused by improper garbage collection.
This problem was reported by lot of people and I documented it recently in this StackOverflow question.

What I found out is that the reason for it was the improper cleanup after each test which is never mentioned anywhere even in the AngularJs testing guide.
Example:

describe('Test suite', function() {
  var a, b;

  beforeEach(function() {
    // init heavy objects
    a = new Array(10000).join('x');
    b = new Array(10000).join('y');
  });

  // NOTE: often forgotten cleanup
  afterEach(function () {
    a = null;
    b = null;
  });

  it('spec1', function() {
    // spec code..
  });

  // other specs and inner suites ..
});

If the needed cleanup is forgotten because jasmine builds up a tree from all registered suits and each suites containes references to his beforeEach, afterEach.. functions which contain references to the describe function closure which holds references to the 'a' and 'b' variables, the large objects that are referenced by that variables won't be GC-ed until Jasmine stops the execution.

We can't say that this is an issue with Jasmine, the problem is because this information is not well transferred to all the developers using this great framework. The question is how we can improve this? Is there a better way that enables easy access to the defined variables in every inner spec, suite, function.. as this one but without caring for the cleanup which is almost always forgotten?

My idea for a workaround was to define only one 'container' object (ex. 'suite') which is nulled in an afterEach block and attach all variables there. This in the same time reduces the code length and keeps us safe from forgetting some defined variable.
Example:

describe('Test suite', function() {
  var suite = {};

  beforeEach(function() {
    // init heavy objects
    suite.a = new Array(10000).join('x');
    suite.b = new Array(10000).join('y');
  });

  // NOTE: often forgotten cleanup
  afterEach(function () {
    suite = null;
  });

  // other specs and inner suites ..
});

[ragaskar]

Thanks for this detailed email Sashe, I'm sure it will help people who are
having memory leak issues.

R.

On Sun, Oct 11, 2015 at 6:03 AM, Sashe Klechkovski <notifications@github.com

wrote:

Hi, this is the first issue that I am creating after almost 2 years of
usage of your extraordinary testing framework for JavaScript. I am using it
mainly for testing AngularJs code and I love it, it enables me to test
every peace of my code and with the BDD syntax every test simply becomes
part of my code documentation.

Recently after some extensive debugging, reading and analysing your source
code I finally found out why I was experiencing memory leak problems. The
problem was that the running tests were not able to finish because of the
build up memory consumption in the browser caused by improper garbage
collection.
This problem was reported by lot of people and I documented it recently in
this
http://stackoverflow.com/questions/32998442/angularjs-unit-testing-memory-leaks
StackOverflow question.

What I found out is that the reason for it was the improper cleanup after
each test which is never mentioned anywhere even in the AngularJs testing
guide https://docs.angularjs.org/guide/unit-testing.
Example:

describe('Test suite', function() {
var a, b;

beforeEach(function() {
// init heavy objects
a = new Array(10000).join('x');
b = new Array(10000).join('y');
});

// NOTE: often forgotten cleanup
afterEach(function () {
a = null;
b = null;
});

it('spec1', function() {
// spec code..
});

// other specs and inner suites ..
});

If the needed cleanup is forgotten because jasmine builds up a tree from
all registered suits and each suites containes references to his
beforeEach, afterEach.. functions which contain references to the describe
function closure which holds references to the 'a' and 'b' variables, the
large objects that are referenced by that variables won't be GC-ed until
Jasmine stops the execution.

We can't say that this is an issue with Jasmine, the problem is because
this information is not well transferred to all the developers using this
great framework. The question is how we can improve this? Is there a better
way that enables easy access to the defined variables in every inner spec,
suite, function.. as this one but without caring for the cleanup which is
almost always forgotten?

My idea for a workaround was to define only one 'container' object (ex.
'suite') which is nulled in an afterEach block and attach all variables
there. This in the same time reduces the code length and keeps us safe from
forgetting some defined variable.
Example:

describe('Test suite', function() {
var suite = {};

beforeEach(function() {
// init heavy objects
suite.a = new Array(10000).join('x');
suite.b = new Array(10000).join('y');
});

// NOTE: often forgotten cleanup
afterEach(function () {
suite = null;
});

// other specs and inner suites ..
});

—
Reply to this email directly or view it on GitHub
#941.

[slackersoft]

You might also look at putting your variables onto the this keyword (http://jasmine.github.io/edge/introduction.html#section-The_this_keyword) which will get disposed of when done.

[slackersoft]

Closing this since it doesn't necessarily sound like an issue with jasmine itself, and jasmine provides a potential workaround with the this keyword.

Thanks for using jasmine!

[codeword]

Whoa, hold on, @slackersoft, are you saying the pattern of

describe("A suite is just a function", function() {
  var a;

  it("and so is a spec", function() {
    a = true;

    expect(a).toBe(true);
  });
});

causes memory leaks in the testing framework and that we are ok with that? I am really surprised by that as this pattern is all over the introduction to Jasmine docs and has always been the pattern for writing tests. If this pattern is in fact leaky then it is my opinion that Jasmine needs to be very clear in their documentation of the dangers of declaring variables in their describe blocks and update all their documentation to use some non leaky method of sharing variables between blocks. In my opinion it should not be expected that developers create afterEach blocks for all their describes to ensure they deallocate all memory they may have used.

Honestly I think the right thing for Jasmine to do is to find the cause of the leak and fix it so that developers can use described scoped variables and know they go away when the describe block ends.

[thedocbwarren]

I've experienced this in my testing many times. In fact in my larger test suites it builds up a large amount of memory that appears to eventually slow down the browser.

I've performed before and after methods to attempt deallocation as well with some benefit but not a fix.

Sent from my iPhone

On Jun 23, 2016, at 8:39 AM, Jonathan Barnes notifications@github.com wrote:

Whoa, hold on, @slackersoft, are you saying the pattern of

describe("A suite is just a function", function() {
var a;

it("and so is a spec", function() {
a = true;

expect(a).toBe(true);

});
});
causes memory leaks in the testing framework and that we are ok with that? I am really surprised by that as this pattern is all over the introduction to Jasmine docs and has always been the pattern for writing tests. If this pattern is in fact leaky then it is my opinion that Jasmine needs to be very clear in their documentation of the dangers of declaring variables in their describe blocks and update all their documentation to use some non leaky method of sharing variables between blocks. In my opinion it should not be expected that developers create afterEach blocks for all their describes to ensure they deallocate all memory they may have used.

Honestly I think the right thing for Jasmine to do is to find the cause of the leak and fix it so that developers can use described scoped variables and know they go away when the describe block ends.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[Gerg]

These lines of code in Jasmine are meant to encourage your JS engine to garbage collect your closures:

jasmine/src/core/Env.js

Lines 158 to 166 in f6da084

	function clearStack(fn) {
	currentSpecCallbackDepth++;
	if (currentSpecCallbackDepth >= maximumSpecCallbackDepth) {
	currentSpecCallbackDepth = 0;
	realSetTimeout(fn, 0);
	} else {
	fn();
	}
	}

That said, Jasmine can only do so much to free memory. Memory leaks in your source code, its dependencies, or your spec runner (see #1108) will compound over your suite's execution. Also, if you have circular references in your code, then it may never be freed (see here).

Jasmine is definitely not perfect, so if you are able to find leaks in Jasmine or strategies to mitigate leaks, we would love to hear about them.

[firezdog]

What if you limited nesting of tests to cases where absolutely necessary, implementing proper dereferencing as needed?

[saskodh]

@firezdog, I found this workaround very useful, I haven't run on a memory leak issue since I started using it and updated all the tests.
https://stackoverflow.com/a/33072532/4319253

[slackersoft]

Newer versions of Jasmine (3.0 and later I think) should also do what they can to remove references to functions (specs, before, after, etc) that have been fully executed, so we should be freeing up as much memory as we can.