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

Commit 824b2a2

Browse files
committed
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
1 parent cd8dd61 commit 824b2a2

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
@@ -1183,7 +1183,10 @@ function $RootScopeProvider() {
11831183
return function() {
11841184
var indexOfListener = namedListeners.indexOf(listener);
11851185
if (indexOfListener !== -1) {
1186-
namedListeners[indexOfListener] = null;
1186+
// Use delete in the hope of the browser deallocating the memory for the array entry,
1187+
// while not shifting the array indexes of other listeners.
1188+
// See issue https://github.com/angular/angular.js/issues/16135
1189+
delete namedListeners[indexOfListener];
11871190
decrementListenerCount(self, 1, name);
11881191
}
11891192
};

test/ng/rootScopeSpec.js

+15
Original file line numberDiff line numberDiff line change
@@ -2316,6 +2316,21 @@ describe('Scope', function() {
23162316
}));
23172317

23182318

2319+
// See issue https://github.com/angular/angular.js/issues/16135
2320+
it('should deallocate the listener array entry', inject(function($rootScope) {
2321+
var remove1 = $rootScope.$on('abc', noop);
2322+
$rootScope.$on('abc', noop);
2323+
2324+
expect($rootScope.$$listeners['abc'].length).toBe(2);
2325+
expect(0 in $rootScope.$$listeners['abc']).toBe(true);
2326+
2327+
remove1();
2328+
2329+
expect($rootScope.$$listeners['abc'].length).toBe(2);
2330+
expect(0 in $rootScope.$$listeners['abc']).toBe(false);
2331+
}));
2332+
2333+
23192334
it('should call next listener when removing current', inject(function($rootScope) {
23202335
var listener1 = jasmine.createSpy().and.callFake(function() { remove1(); });
23212336
var remove1 = $rootScope.$on('abc', listener1);

0 commit comments

Comments
 (0)