Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 97d0224

Browse files
jbedardNarretz
authored andcommitted
fix($rootScope): fix potential memory leak when removing scope listeners
When removing listeners they are removed from the array but the array size is not changed until the event is fired again. If the event is never fired but listeners are added/removed then the array will continue growing. By changing the listener removal to `delete` the array entry instead of setting it to `null` browsers can potentially deallocate the memory for the entry. Fixes #16135 Closes #16161
1 parent e5fb929 commit 97d0224

File tree

2 files changed

+19
-1
lines changed

2 files changed

+19
-1
lines changed

src/ng/rootScope.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -1273,7 +1273,10 @@ function $RootScopeProvider() {
12731273
return function() {
12741274
var indexOfListener = namedListeners.indexOf(listener);
12751275
if (indexOfListener !== -1) {
1276-
namedListeners[indexOfListener] = null;
1276+
// Use delete in the hope of the browser deallocating the memory for the array entry,
1277+
// while not shifting the array indexes of other listeners.
1278+
// See issue https://github.com/angular/angular.js/issues/16135
1279+
delete namedListeners[indexOfListener];
12771280
decrementListenerCount(self, 1, name);
12781281
}
12791282
};

test/ng/rootScopeSpec.js

+15
Original file line numberDiff line numberDiff line change
@@ -2438,6 +2438,21 @@ describe('Scope', function() {
24382438
}));
24392439

24402440

2441+
// See issue https://github.com/angular/angular.js/issues/16135
2442+
it('should deallocate the listener array entry', inject(function($rootScope) {
2443+
var remove1 = $rootScope.$on('abc', noop);
2444+
$rootScope.$on('abc', noop);
2445+
2446+
expect($rootScope.$$listeners['abc'].length).toBe(2);
2447+
expect(0 in $rootScope.$$listeners['abc']).toBe(true);
2448+
2449+
remove1();
2450+
2451+
expect($rootScope.$$listeners['abc'].length).toBe(2);
2452+
expect(0 in $rootScope.$$listeners['abc']).toBe(false);
2453+
}));
2454+
2455+
24412456
it('should call next listener after removing the current listener via its own handler', inject(function($rootScope) {
24422457
var listener1 = jasmine.createSpy('listener1').and.callFake(function() { remove1(); });
24432458
var remove1 = $rootScope.$on('abc', listener1);

0 commit comments

Comments
 (0)