From 844280eaa5457d1293c1725ac85f22f355f0d180 Mon Sep 17 00:00:00 2001 From: Andrew Henry Date: Fri, 26 Apr 2019 10:34:24 -0700 Subject: [PATCH] Memory leak fixes (#2387) * Clean up listeners * Fix uses of 'destroy' instead of 'destroyed' --- src/adapter/views/LegacyViewProvider.js | 22 ++++++++++++++----- .../views/TypeInspectorViewProvider.js | 12 ++++++++-- src/plugins/telemetryTable/TelemetryTable.js | 2 +- src/ui/layout/tree-item.vue | 5 +++-- src/ui/mixins/context-menu-gesture.js | 2 +- src/ui/preview/Preview.vue | 2 +- 6 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/adapter/views/LegacyViewProvider.js b/src/adapter/views/LegacyViewProvider.js index 9dddabd2ce..78727c8c04 100644 --- a/src/adapter/views/LegacyViewProvider.js +++ b/src/adapter/views/LegacyViewProvider.js @@ -45,22 +45,27 @@ define([ view: function (domainObject) { let $rootScope = openmct.$injector.get('$rootScope'); let templateLinker = openmct.$injector.get('templateLinker'); - let scope = $rootScope.$new(); + let scope = $rootScope.$new(true); let legacyObject = convertToLegacyObject(domainObject); let isDestroyed = false; let unlistenToStatus; + let element; scope.domainObject = legacyObject; scope.model = legacyObject.getModel(); - + let child; + let parent; return { show: function (container) { + parent = container; + child = document.createElement('div'); + parent.appendChild(child); let statusCapability = legacyObject.getCapability('status'); unlistenToStatus = statusCapability.listen((newStatus) => { - container.classList.remove('s-status-timeconductor-unsynced'); + child.classList.remove('s-status-timeconductor-unsynced'); if (newStatus.includes('timeconductor-unsynced')) { - container.classList.add('s-status-timeconductor-unsynced'); + child.classList.add('s-status-timeconductor-unsynced'); } }); @@ -84,12 +89,13 @@ define([ uses.forEach(function (key, i) { scope[key] = results[i]; }); + element = openmct.$angular.element(child); templateLinker.link( scope, - openmct.$angular.element(container), + element, legacyView ); - container.classList.add('u-contents'); + child.classList.add('u-contents'); } if (promises.length) { @@ -103,7 +109,11 @@ define([ } }, destroy: function () { + element.off(); + element.remove(); scope.$destroy(); + element = null; + scope = null; unlistenToStatus(); } } diff --git a/src/adapter/views/TypeInspectorViewProvider.js b/src/adapter/views/TypeInspectorViewProvider.js index d427033fa2..1597555fcc 100644 --- a/src/adapter/views/TypeInspectorViewProvider.js +++ b/src/adapter/views/TypeInspectorViewProvider.js @@ -41,15 +41,18 @@ define([ let domainObject = selection[0][0].context.item; let $rootScope = openmct.$injector.get('$rootScope'); let templateLinker = openmct.$injector.get('templateLinker'); - let scope = $rootScope.$new(); + let scope = $rootScope.$new(true); let legacyObject = convertToLegacyObject(domainObject); let isDestroyed = false; + let element; scope.domainObject = legacyObject; scope.model = legacyObject.getModel(); return { show: function (container) { + let child = document.createElement('div'); + container.appendChild(child); // TODO: implement "gestures" support ? let uses = representation.uses || []; let promises = []; @@ -70,9 +73,10 @@ define([ uses.forEach(function (key, i) { scope[key] = results[i]; }); + element = openmct.$angular.element(child) templateLinker.link( scope, - openmct.$angular.element(container), + element, representation ); container.style.height = '100%'; @@ -89,7 +93,11 @@ define([ } }, destroy: function () { + element.off(); + element.remove(); scope.$destroy(); + element = null; + scope = null; } } } diff --git a/src/plugins/telemetryTable/TelemetryTable.js b/src/plugins/telemetryTable/TelemetryTable.js index 9a015d8f8a..6a4787a55d 100644 --- a/src/plugins/telemetryTable/TelemetryTable.js +++ b/src/plugins/telemetryTable/TelemetryTable.js @@ -239,7 +239,7 @@ define([ this.filteredRows.destroy(); Object.keys(this.subscriptions).forEach(this.unsubscribe, this); this.openmct.time.off('bounds', this.refreshData); - this.openmct.time.on('timeSystem', this.refreshData); + this.openmct.time.off('timeSystem', this.refreshData); if (this.filterObserver) { this.filterObserver(); } diff --git a/src/ui/layout/tree-item.vue b/src/ui/layout/tree-item.vue index e47a11e884..398eeeae70 100644 --- a/src/ui/layout/tree-item.vue +++ b/src/ui/layout/tree-item.vue @@ -70,17 +70,18 @@ this.domainObject = newObject; }); this.$once('hook:destroyed', removeListener); + if (this.openmct.composition.get(this.node.object)) { this.hasChildren = true; } this.openmct.router.on('change:path', this.highlightIfNavigated); }, - destroy() { + destroyed() { + this.openmct.router.off('change:path', this.highlightIfNavigated); if (this.composition) { this.composition.off('add', this.addChild); this.composition.off('remove', this.removeChild); - this.openmct.router.off('change:path', this.highlightIfNavigated); delete this.composition; } }, diff --git a/src/ui/mixins/context-menu-gesture.js b/src/ui/mixins/context-menu-gesture.js index a88b563dcd..7cf11281ee 100644 --- a/src/ui/mixins/context-menu-gesture.js +++ b/src/ui/mixins/context-menu-gesture.js @@ -18,7 +18,7 @@ export default { this.objectPath.forEach(object => { if (object) { - this.$once('hook:destroy', + this.$once('hook:destroyed', this.openmct.objects.observe(object, '*', updateObject.bind(this, object))) } }); diff --git a/src/ui/preview/Preview.vue b/src/ui/preview/Preview.vue index ba70717c4e..70800ecb59 100644 --- a/src/ui/preview/Preview.vue +++ b/src/ui/preview/Preview.vue @@ -116,7 +116,7 @@ this.notebookEnabled = true; } }, - destroy() { + destroyed() { this.view.destroy(); } }