From b3023b6d92115820115852c996f99be5fac6f947 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Tue, 8 Aug 2017 23:24:17 -0700 Subject: [PATCH] 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 --- src/ng/rootScope.js | 5 ++++- test/ng/rootScopeSpec.js | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 85c04376b2f5..f2fbec1a1c61 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -1207,7 +1207,10 @@ function $RootScopeProvider() { return function() { var indexOfListener = namedListeners.indexOf(listener); if (indexOfListener !== -1) { - namedListeners[indexOfListener] = null; + // Use delete in the hope of the browser deallocating the memory for the array entry, + // while not shifting the array indexes of other listeners. + // See issue https://github.com/angular/angular.js/issues/16135 + delete namedListeners[indexOfListener]; decrementListenerCount(self, 1, name); } }; diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index dd1a23f20625..83005e674cbc 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -1903,6 +1903,21 @@ describe('Scope', function() { })); + // See issue https://github.com/angular/angular.js/issues/16135 + it('should deallocate the listener array entry', inject(function($rootScope) { + var remove1 = $rootScope.$on('abc', noop); + $rootScope.$on('abc', noop); + + expect($rootScope.$$listeners['abc'].length).toBe(2); + expect(0 in $rootScope.$$listeners['abc']).toBe(true); + + remove1(); + + expect($rootScope.$$listeners['abc'].length).toBe(2); + expect(0 in $rootScope.$$listeners['abc']).toBe(false); + })); + + it('should call next listener after removing the current listener via its own handler', inject(function($rootScope) { var listener1 = jasmine.createSpy('listener1').and.callFake(function() { remove1(); }); var remove1 = $rootScope.$on('abc', listener1);