Memory Leaks

When it comes to unit testing in AngularJS there are many things that, developers like us can and will do wrong. The most crucial one is to create memory leaks in our unit tests, which mostly result either in a crash of the unit test runner (browser crash) or in creating a coherence/dependency between different tests that are meant to be separated. This article shows the basic mistakes that are done out in the wild and illustrates the rather easy fix.

Beware as this article only covers the analysis of the Jasmine testing framework. I didn’t invest any time in checking what happens in other frameworks like Mocha.

 

tl;dr;

• Do not declare any var outside of a ‘it’-block unless you null it manually after the test
• Declare shared variables on this, those variables are shared between: beforeEach,
  afterEach and it and are properly cleaned up by Jasmine
• if you use $compile, call remove() on the created element when you are done testing it (e.g. in an afterEach)

 

How we noticed there was a memory leak

When you’re starting a fresh project you are unlikely to run into memory leaks. I found out about the problems when I joined another project with more than 3.500 unit tests. I write Angular applications for about 2-3 years now, but when we discovered what we were doing wrong in our unit tests it got me thinking…

Heres how we noticed there is a problem. It has been fairly simple: the browser, that executed the unit tests crashed. We were using Firefox as a runner, since PhantomJS quits its job about a year ago with ~1.000 unit tests and Chrome did so a while later. This time Firefox started to crash as well, we seemed to hit the max. capabilities of Firefox as well. There was no other choice then to find out what exactly broke the unit tests. We figured out that it must had something with memory leaks to do, because the browser ate to much memory until it collapsed. It also showed this strange behaviour that it would make huge pauses between some tests and the pauses then became longer, the more tests were executed.

After using Google for while we found a sample projects that illustrates two ways angular unit tests can leak memory. So I made a fork of it and tried to find a suitable solution that fits our needs

https://github.com/kaihenzler/ng-unit-testing-leak

Setting up a sample app

The sample app essentially contains only one directive and one service. What the directive does consists of just requesting a very big Object from the service and using ng-repeat to loop over this huge object.

To explain the issue we checked out the GitHub-Repo mentioned above and run a couple of commands. First of all we started with npm install, followed by bower install. Then, we started the unit tests by typing ./node_modules/karma/bin/karma start karma.conf.js in the console.
After about a minute we have seen that we executed 3.000 unit tests.

To get the tests to fail we need to edit 2 files:

• We uncomment the lines 47-49 in test/directives/heavyLoad-directive.spec.js and
• comment the tests in line 79-81 of heavyLoad-directive.using-this.spec.js.

(what we do, is just disabling “valid” unit test and enabling the faulty one..  we’ll explain more details later).

A second run of ./node_modules/karma/bin/karma start karma.conf.js now gives us the following result:

Why does the test-run fail?

To find out why the test is killing our browser we launched the test suite in Google Chrome. At some point (test was running since ~30 sec.) we set a break point (to cause the test suite routine to pause) and then we opened the profile-tab of Chrome’s DevTools to take a Heap Snapshot.

When we took a quick look at this Heap Snapshot I was really surprised on how big it was. I expected not more than (maybe) ~25 MB of memory usage. But as Note 1 marks we reached over 800 MB in about 30 sec.

Note 2 showed us where this huge pile of memory usage came from. We can see from that picture that the 92% of the memory has been used by some arrays…you might be wondering: “where are we storing such a huge amount of arrays in our little demo application?”

Note 3 shows us that the stored arrays look just like the ones returned from our service, that generates huge data structures.

You have to trust me, that there is not only one Object (like the one marked in red) but there are literally thousands of them.
This can mean only one thing: Our unit tests seem not to cleanup properly and somehow keep references of previous test runs in memory.

let’s have a look at the test code which causes this strange behaviour. Previously we executed this ‘describe’-block 1.000 times, in order to make the memory leak to appear.

There are a few things to notice here, in that, this layout of the test follows the official angular unit testing guideline where we:

• declare shared variables inside the describe block
• instantiate the app in a beforeEach
• inject Components in a beforeEach
• use an helper function to e.g. compile a directive
• have one or multiple ‘it’-blocks that test various things and use those declared vars and functions

Unfortunately something here seems to be wrong. As we saw in the previous chapter, where we took a look at the Chrome Profiler, none of the shared variables seem to be cleared after the execution of a ‘it’-block. That’s because Jasmine builds up a tree from all registered test suites and each suite contains references to the beforeEach- and afterEach-functions. Those references, in turn, contain other references to the described function closure which holds other references to the shared variables…and the large objects that are referenced by that variables won’t be GC-ed (Garbage Collected) until Jasmine stops the execution.

This may sound confusing at a first glance: let me put it in simple words. If a variable, that is defined inside a ‘describe’-block has a value assigned (which is not null or undefined) the Garbage Collector of your Browser is not able to cleanup the related memory, because Jasmine internally holds a reference to that block.

Fixing the Unit Tests

As I mentioned before, we have to set all those variables declared inside each describe to null. Let’s do it:

If we would run our unit-tests again, they shouldn’t crash again, right? Unfortunately they still do.
After an harder debugging session it became clearer that the usage of $compile seemed to be the main cause of the memory leak. In fact, after attaching a debugger we saw that $compile generates a DocumentFragment (we can think of it as a light-weight Document). As we didn’t attach the DocumentFragment to the actual DOM, we cannot see our compiled template in the Elements-Inspector when we debug our tests, but what we can do is to search in the Heap Snapshot for occurrences of DocumentFragment.

And as we can see, there are a lot of DocumentFragments laying around. You can guess what is about to do next: we take care of proper cleanup of the $compile in our unit test.

A deeper look at the Chrome Profile from above showed, that these elements were somehow linked to a jQuery cache. I figured that I might dive into angular’s source code. Unsurprisingly it helped a lot. In some of angular’s directive tests they cleanup the DOM in an afterEach-block, where they call a helper function ‘dealoc’ from their testHelpers and it indeed does some cleanup of cache-related jQuery things.

I didn’t spent a lot of time but went to the angular-bootstrap repository, which is a great source for good angular-code. In the first directive spec I looked at (the accordion), I noticed that they remove the element from the DOM in an afterEach block.

So that’s what we’ll do as well: we add one line and remove the element.

and finally, our unit tests run gracefully without any hiccups. But is that what we want to do? Cleanup every little bit of what we defined? Read further and I’ll show another approach to Jasmine unit tests!

How to make your tests look even cleaner

Let me introduce you a well known paradigm in almost every programming language: Jasmine’s this. Jasmine provides a this which can be used to share variables between beforeEach, afterEach and it. Isn’t it exactly the same that we wanted to accomplish with our test setup? (as we defined a var inside a describe block, in order to let the beforeEach and it-blocks accessing those vars). In my opinion it is! Let me show you how this test could look like, if it’s written with this-syntax.

advantages of this-syntax:

• Jasmine takes care of properly cleaning up everything that is defined on this
• You can directly write-to and read-from this, no need to setup a var on the right function scope for all those it-blocks, beforeEach and afterEach blocks to be accessible.
• Someone will always forget to write cleanup code, because it is very hard to locate where a manual cleanup might be necessary.
• Why reinvent the wheel, when Jasmine gives us exactly what we need.

disadvantages of this-syntax:

• You have to go through every single file and possibly change every 2nd line (although search/replace is your friend).
• Most unit tests would only need one additional afterEach block where you could null all vars.
• this has a bad reputation in the JavaScript community because it appears to do weird things sometimes.

It’s not up to me to decide what you shall end up doing.
IMHO the this-syntax seemed to make the things easier, because I tend to forget to write the proper cleanup code in unit tests.

A huge shoutout goes to Sashe Klechkovski, who came up with the demo-app and brought me to the suggestion of this-syntax.