Skip to content

Commit 44684c9

Browse files
committed
fix(ngMock): prevent memory leak due to data attached to $rootElement
Starting with 88bb551, `ngMock` will attach the `$injector` to the `$rootElement`, but will never clean it up, resulting in a memory leak. Since a new `$rootElement` is created for every test, this leak causes Karma to crash on large test-suites. The problem was not detected by our internal tests, because we do our own clean-up in `testabilityPatch.js`. This commit prevents the memory leak, by cleaning up all data attached to `$rootElement` after each test. Fixes angular#14094
1 parent 2d6c218 commit 44684c9

File tree

2 files changed

+145
-8
lines changed

2 files changed

+145
-8
lines changed

src/ngMock/angular-mocks.js

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,12 @@ angular.mock.$Browser = function() {
127127
};
128128
angular.mock.$Browser.prototype = {
129129

130-
/**
131-
* @name $browser#poll
132-
*
133-
* @description
134-
* run all fns in pollFns
135-
*/
130+
/**
131+
* @name $browser#poll
132+
*
133+
* @description
134+
* run all fns in pollFns
135+
*/
136136
poll: function poll() {
137137
angular.forEach(this.pollFns, function(pollFn) {
138138
pollFn();
@@ -2089,9 +2089,11 @@ angular.mock.$RAFDecorator = ['$delegate', function($delegate) {
20892089
/**
20902090
*
20912091
*/
2092+
var originalRootElement;
20922093
angular.mock.$RootElementProvider = function() {
20932094
this.$get = ['$injector', function($injector) {
2094-
return angular.element('<div ng-app></div>').data('$injector', $injector);
2095+
originalRootElement = angular.element('<div ng-app></div>').data('$injector', $injector);
2096+
return originalRootElement;
20952097
}];
20962098
};
20972099

@@ -2600,7 +2602,17 @@ if (window.jasmine || window.mocha) {
26002602
currentSpec = null;
26012603

26022604
if (injector) {
2603-
injector.get('$rootElement').off();
2605+
var cleanUpElems = [originalRootElement];
2606+
var $rootElement = injector.get('$rootElement');
2607+
if ($rootElement !== originalRootElement) cleanUpElems.push($rootElement);
2608+
cleanUpElems.forEach(function(elem) {
2609+
// The `$rootElement` might not have been created or
2610+
// a mocked `$rootElement` might not have the standard methods
2611+
if (elem && elem.off) elem.off();
2612+
if (elem && elem.removeData) elem.removeData();
2613+
});
2614+
if (window.jQuery) window.jQuery.cleanData(cleanUpElems);
2615+
26042616
injector.get('$rootScope').$destroy();
26052617
}
26062618

test/ngMock/angular-mocksSpec.js

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2408,9 +2408,134 @@ describe('ngMockE2E', function() {
24082408
});
24092409
});
24102410

2411+
24112412
describe('make sure that we can create an injector outside of tests', function() {
24122413
//since some libraries create custom injectors outside of tests,
24132414
//we want to make sure that this is not breaking the internals of
24142415
//how we manage annotated function cleanup during tests. See #10967
24152416
angular.injector([function($injector) {}]);
24162417
});
2418+
2419+
2420+
describe('don\'t leak memory between tests', function() {
2421+
var jQuery = window.jQuery;
2422+
var prevRootElement;
2423+
var prevRemoveDataSpy;
2424+
var prevCleanDataSpy;
2425+
2426+
it('should run a test to keep track of `removeData()`/`cleanData()` calls for later inspection',
2427+
function() {
2428+
module(function($provide) {
2429+
$provide.decorator('$rootElement', function($delegate) {
2430+
// Spy on `.removeData()` and `jQuery.cleanData()`,
2431+
// so the next test can verify that they have been called as necessary
2432+
prevRootElement = $delegate;
2433+
prevRemoveDataSpy = spyOn($delegate, 'removeData').andCallThrough();
2434+
2435+
if (jQuery) {
2436+
prevCleanDataSpy = spyOn(jQuery, 'cleanData').andCallThrough();
2437+
}
2438+
2439+
return $delegate;
2440+
});
2441+
});
2442+
2443+
// Inject the `$rootElement` to ensure it has been created
2444+
inject(function($rootElement) {
2445+
expect($rootElement.injector()).toBeDefined();
2446+
});
2447+
}
2448+
);
2449+
2450+
it('should clean up `$rootElement`-related data after each test', function() {
2451+
// One call is made by `testabilityPatch`'s `dealoc()`
2452+
// We want to verify the subsequent call, made by `angular-mocks`
2453+
// (Since `testabilityPatch` re-wrapps `$rootElement` and because jQuery
2454+
// returns a different object, we don't capture the 1st call when using jQuery)
2455+
expect(prevRemoveDataSpy.callCount).toBe(jQuery ? 1 : 2);
2456+
2457+
if (jQuery) {
2458+
// One call is made by `testabilityPatch`'s `dealoc()`
2459+
// We want to verify the subsequent call, made by `angular-mocks`
2460+
expect(prevCleanDataSpy.callCount).toBe(2);
2461+
2462+
var cleanUpElems = prevCleanDataSpy.calls[1].args[0];
2463+
expect(cleanUpElems.length).toBe(1);
2464+
expect(cleanUpElems[0][0]).toBe(prevRootElement[0]);
2465+
}
2466+
});
2467+
});
2468+
2469+
describe('don\'t leak memory between tests with mocked `$rootScope`', function() {
2470+
var jQuery = window.jQuery;
2471+
var prevOriginalRootElement;
2472+
var prevOriginalRemoveDataSpy;
2473+
var prevRootElement;
2474+
var prevRemoveDataSpy;
2475+
var prevCleanDataSpy;
2476+
2477+
it('should run a test to keep track of `removeData()`/`cleanData()` calls for later inspection',
2478+
function() {
2479+
module(function($provide) {
2480+
$provide.decorator('$rootElement', function($delegate) {
2481+
// Mock `$rootElement` to be able to verify that the correct object is cleaned up
2482+
var mockRootElement = angular.element('<div></div>');
2483+
2484+
// Spy on `.removeData()` and `jQuery.cleanData()`,
2485+
// so the next test can verify that they have been called as necessary
2486+
prevOriginalRootElement = $delegate;
2487+
prevOriginalRemoveDataSpy = spyOn($delegate, 'removeData').andCallThrough();
2488+
2489+
prevRootElement = mockRootElement;
2490+
prevRemoveDataSpy = spyOn(mockRootElement, 'removeData').andCallThrough();
2491+
2492+
if (jQuery) {
2493+
prevCleanDataSpy = spyOn(jQuery, 'cleanData').andCallThrough();
2494+
}
2495+
2496+
return mockRootElement;
2497+
});
2498+
});
2499+
2500+
// Inject the `$rootElement` to ensure it has been created
2501+
inject(function($rootElement) {
2502+
expect($rootElement).toBe(prevRootElement);
2503+
expect(prevOriginalRootElement.injector()).toBeDefined();
2504+
expect(prevRootElement.injector()).toBeUndefined();
2505+
2506+
// If we don't clean up `prevOriginalRootElement`-related data now, `testabilityPatch` will
2507+
// complain about a memory leak, because it doesn't clean up after the original
2508+
// `$rootElement`.
2509+
// This is a false alarm, because `angular-mocks` will clean up later.
2510+
prevOriginalRootElement.removeData();
2511+
prevOriginalRemoveDataSpy.reset();
2512+
2513+
expect(prevOriginalRemoveDataSpy.callCount).toBe(0);
2514+
});
2515+
}
2516+
);
2517+
2518+
it('should clean up after the `$rootElement` (both original and decorated) after each test',
2519+
function() {
2520+
// Only `angular-mocks` cleans up after the original `$rootElement`, not `testabilityPatch`
2521+
expect(prevOriginalRemoveDataSpy.callCount).toBe(1);
2522+
2523+
// One call is made by `testabilityPatch`'s `dealoc()`
2524+
// We want to verify the subsequent call, made by `angular-mocks`
2525+
// (Since `testabilityPatch` re-wrapps `$rootElement` and because jQuery
2526+
// returns a different object, we don't capture the 1st call when using jQuery)
2527+
expect(prevRemoveDataSpy.callCount).toBe(jQuery ? 1 : 2);
2528+
2529+
if (jQuery) {
2530+
// One call is made by `testabilityPatch`'s `dealoc()`
2531+
// We want to verify the subsequent call, made by `angular-mocks`
2532+
expect(prevCleanDataSpy.callCount).toBe(2);
2533+
2534+
var cleanUpElems = prevCleanDataSpy.calls[1].args[0];
2535+
expect(cleanUpElems.length).toBe(2);
2536+
expect(cleanUpElems[0][0]).toBe(prevOriginalRootElement[0]);
2537+
expect(cleanUpElems[1][0]).toBe(prevRootElement[0]);
2538+
}
2539+
}
2540+
);
2541+
});

0 commit comments

Comments
 (0)