Merge pull request #18767 from emberjs/fix-mem-leak

[BUGFIX release] Fix observer leaks

packages/@ember/-internals/meta/index.ts

@@ -1,10 +1 @@
-export {
-  counters,
-  deleteMeta,
-  Meta,
-  meta,
-  MetaCounters,
-  peekMeta,
-  setMeta,
-  UNDEFINED,
-} from './lib/meta';
+export { counters, Meta, meta, MetaCounters, peekMeta, setMeta, UNDEFINED } from './lib/meta';

packages/@ember/-internals/meta/lib/meta.ts

@@ -156,6 +156,10 @@ export class Meta {
   }
 
   destroy() {
+    if (DEBUG) {
+      counters!.deleteCalls++;
+    }
+
     if (this.isMetaDestroyed()) {
       return;
     }
@@ -647,34 +651,6 @@ export function peekMeta(obj: object): Meta | null {
   return null;
 }
 
-/**
-  Tears down the meta on an object so that it can be garbage collected.
-  Multiple calls will have no effect.
-
-  @method deleteMeta
-  @for Ember
-  @param {Object} obj  the object to destroy
-  @return {void}
-  @private
-*/
-export function deleteMeta(obj: object) {
-  assert('Cannot call `deleteMeta` on null', obj !== null);
-  assert('Cannot call `deleteMeta` on undefined', obj !== undefined);
-  assert(
-    `Cannot call \`deleteMeta\` on ${typeof obj}`,
-    typeof obj === 'object' || typeof obj === 'function'
-  );
-
-  if (DEBUG) {
-    counters!.deleteCalls++;
-  }
-
-  let meta = peekMeta(obj);
-  if (meta !== null) {
-    meta.destroy();
-  }
-}
-
 /**
   Retrieves the meta hash for an object. If `writable` is true ensures the
   hash is writable for this object as well.

packages/@ember/-internals/metal/index.ts

@@ -50,7 +50,15 @@ export { default as getProperties } from './lib/get_properties';
 export { default as setProperties } from './lib/set_properties';
 export { default as expandProperties } from './lib/expand_properties';
 
-export { addObserver, activateObserver, removeObserver, flushAsyncObservers } from './lib/observer';
+export { destroy } from './lib/destroy';
+export {
+  ASYNC_OBSERVERS,
+  SYNC_OBSERVERS,
+  addObserver,
+  activateObserver,
+  removeObserver,
+  flushAsyncObservers,
+} from './lib/observer';
 export { Mixin, aliasMethod, mixin, observer, applyMixin } from './lib/mixin';
 export { default as inject, DEBUG_INJECTION_FUNCTIONS } from './lib/injected_property';
 export { tagForProperty, tagForObject, markObjectAsDirty, CUSTOM_TAG_FOR } from './lib/tags';

packages/@ember/-internals/metal/lib/destroy.ts

@@ -0,0 +1,37 @@
+import { Meta, peekMeta } from '@ember/-internals/meta/lib/meta';
+import { assert } from '@ember/debug';
+import { schedule } from '@ember/runloop';
+import { destroyObservers } from './observer';
+
+/**
+  Enqueues finalization on an object so that it can be garbage collected.
+  Multiple calls will have no effect.
+
+  @method destroy
+  @for Ember
+  @param {Object} obj the object to destroy
+  @return {boolean} true if the object went from not destroying to destroying.
+  @private
+*/
+export function destroy(obj: object): boolean {
+  assert('Cannot call `destroy` on null', obj !== null);
+  assert('Cannot call `destroy` on undefined', obj !== undefined);
+  assert(
+    `Cannot call \`destroy\` on ${typeof obj}`,
+    typeof obj === 'object' || typeof obj === 'function'
+  );
+
+  const m = peekMeta(obj);
+  if (m === null || m.isSourceDestroying()) {
+    return false;
+  }
+  m.setSourceDestroying();
+  destroyObservers(obj);
+  schedule('destroy', m, finalize);
+  return true;
+}
+
+function finalize(this: Meta) {
+  this.setSourceDestroyed();
+  this.destroy();
+}

packages/@ember/-internals/metal/lib/events.ts

@@ -124,8 +124,7 @@ export function sendEvent(
 ) {
   if (actions === undefined) {
     let meta = _meta === undefined ? peekMeta(obj) : _meta;
-    actions =
-      typeof meta === 'object' && meta !== null ? meta.matchingListeners(eventName) : undefined;
+    actions = meta !== null ? meta.matchingListeners(eventName) : undefined;
   }
 
   if (actions === undefined || actions.length === 0) {

packages/@ember/-internals/metal/lib/observer.ts

@@ -15,8 +15,8 @@ interface ActiveObserver {
 }
 
 const SYNC_DEFAULT = !ENV._DEFAULT_ASYNC_OBSERVERS;
-const SYNC_OBSERVERS: Map<object, Map<string, ActiveObserver>> = new Map();
-const ASYNC_OBSERVERS: Map<object, Map<string, ActiveObserver>> = new Map();
+export const SYNC_OBSERVERS: Map<object, Map<string, ActiveObserver>> = new Map();
+export const ASYNC_OBSERVERS: Map<object, Map<string, ActiveObserver>> = new Map();
 
 /**
 @module @ember/object
@@ -153,15 +153,16 @@ export function revalidateObservers(target: object) {
 let lastKnownRevision = 0;
 
 export function flushAsyncObservers(shouldSchedule = true) {
-  if (lastKnownRevision === value(CURRENT_TAG)) {
+  let currentRevision = value(CURRENT_TAG);
+  if (lastKnownRevision === currentRevision) {
     return;
   }
-
-  lastKnownRevision = value(CURRENT_TAG);
+  lastKnownRevision = currentRevision;
 
   ASYNC_OBSERVERS.forEach((activeObservers, target) => {
     let meta = peekMeta(target);
 
+    // if observer target is destroyed remove observers
     if (meta && (meta.isSourceDestroying() || meta.isMetaDestroyed())) {
       ASYNC_OBSERVERS.delete(target);
       return;
@@ -171,7 +172,7 @@ export function flushAsyncObservers(shouldSchedule = true) {
       if (!validate(observer.tag, observer.lastRevision)) {
         let sendObserver = () => {
           try {
-            sendEvent(target, eventName, [target, observer.path]);
+            sendEvent(target, eventName, [target, observer.path], undefined, meta);
           } finally {
             observer.tag = combine(getChainTagsForKey(target, observer.path));
             observer.lastRevision = value(observer.tag);
@@ -205,7 +206,7 @@ export function flushSyncObservers() {
       if (!observer.suspended && !validate(observer.tag, observer.lastRevision)) {
         try {
           observer.suspended = true;
-          sendEvent(target, eventName, [target, observer.path]);
+          sendEvent(target, eventName, [target, observer.path], undefined, meta);
         } finally {
           observer.tag = combine(getChainTagsForKey(target, observer.path));
           observer.lastRevision = value(observer.tag);
@@ -229,3 +230,8 @@ export function setObserverSuspended(target: object, property: string, suspended
     observer.suspended = suspended;
   }
 }
+
+export function destroyObservers(target: object) {
+  if (SYNC_OBSERVERS.size > 0) SYNC_OBSERVERS.delete(target);
+  if (ASYNC_OBSERVERS.size > 0) ASYNC_OBSERVERS.delete(target);
+}

packages/@ember/-internals/metal/tests/accessors/get_test.js

@@ -1,5 +1,6 @@
 import { ENV } from '@ember/-internals/environment';
 import { Object as EmberObject } from '@ember/-internals/runtime';
+import { destroy } from '@ember/-internals/metal';
 import { get, set, getWithDefault, Mixin, observer, computed } from '../..';
 import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
 import { run } from '@ember/runloop';
@@ -196,6 +197,8 @@ moduleFor(
         'foo',
         'should return the set value, not false'
       );
+
+      run(() => destroy(baseObject));
     }
   }
 );
@@ -332,6 +335,8 @@ moduleFor(
         'foo',
         'should return the set value, not false'
       );
+
+      run(() => destroy(baseObject));
     }
 
     ['@test should respect prototypical inheritance when subclasses override CPs'](assert) {

packages/@ember/-internals/metal/tests/alias_test.js

@@ -2,6 +2,7 @@ import {
   alias,
   computed,
   defineProperty,
+  destroy,
   get,
   set,
   addObserver,
@@ -27,7 +28,11 @@ moduleFor(
     }
 
     afterEach() {
-      obj = null;
+      if (obj !== undefined) {
+        destroy(obj);
+        obj = undefined;
+        return runLoopSettled();
+      }
     }
 
     ['@test should proxy get to alt key'](assert) {