From fc7bfe706e649ff8ed97aecabcdff7514598b424 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Wed, 16 Mar 2022 12:26:03 -0600 Subject: [PATCH 1/2] [BUGFIX LTS] remove bad `setFactoryFor` call This call *attempted* to avoid setting the `INIT_FACTORY` on items which did not need it (specifically, it avoided setting it on things which were not instantiable), but ultimately failed to do. It was ultimately setting the new `FactoryManager` instance as the `INIT_FACTORY` value on each instantiable object, which includes classes and not just class instances, since objects are only treated as non-instantiable when they explicitly specify `instantiate: false`. The net was a memory leak: the routing service *class* ended up with an `INIT_FACTORY` pointing to a `FactoryManager` instance which in turn always had a `container` on it, which meant that there was a cycle (the container also referenced the service) and thus a leak. This affects both tests and FastBoot, where we construct new instances of the service whenever we call the `visit` API. --- packages/@ember/-internals/container/lib/container.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/@ember/-internals/container/lib/container.ts b/packages/@ember/-internals/container/lib/container.ts index 4bd3bd1aea8..166b34a1ec5 100644 --- a/packages/@ember/-internals/container/lib/container.ts +++ b/packages/@ember/-internals/container/lib/container.ts @@ -1,5 +1,5 @@ import { Factory, LookupOptions, Owner, setOwner } from '@ember/-internals/owner'; -import { dictionary, HAS_NATIVE_PROXY, HAS_NATIVE_SYMBOL, symbol } from '@ember/-internals/utils'; +import { dictionary, HAS_NATIVE_PROXY, symbol } from '@ember/-internals/utils'; import { assert } from '@ember/debug'; import { assign } from '@ember/polyfills'; import { DEBUG } from '@glimmer/env'; @@ -559,10 +559,6 @@ class FactoryManager { this.madeToString = undefined; this.injections = undefined; setFactoryFor(this, this); - - if (isInstantiatable(container, fullName) && (HAS_NATIVE_SYMBOL || INIT_FACTORY in factory)) { - setFactoryFor(factory, this); - } } toString(): string { From 2590b2bcd80d259f9d36ea4414bc001ef61d7ba3 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Wed, 16 Mar 2022 13:20:44 -0600 Subject: [PATCH 2/2] [BUGFIX beta] remove unneeded `setFactoryFor(this, this)` Identified as part of the work on #20025, this particular `setFactoryFor` was needed at one time but is now defunct. Remove it. --- packages/@ember/-internals/container/lib/container.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@ember/-internals/container/lib/container.ts b/packages/@ember/-internals/container/lib/container.ts index 166b34a1ec5..4cbc19ee869 100644 --- a/packages/@ember/-internals/container/lib/container.ts +++ b/packages/@ember/-internals/container/lib/container.ts @@ -558,7 +558,6 @@ class FactoryManager { this.normalizedName = normalizedName; this.madeToString = undefined; this.injections = undefined; - setFactoryFor(this, this); } toString(): string {