perf(compiler-cli): fix memory leak in retained incremental state (#37835)

Incremental compilation allows for the output state of one compilation to be
reused as input to the next compilation. This involves retaining references
to instances from prior compilations, which must be done carefully to avoid
memory leaks.

This commit fixes such a leak with a complicated retention chain:

* `TrackedIncrementalBuildStrategy` unnecessarily hangs on to the previous
  `IncrementalDriver` (state of the previous compilation) once the current
  compilation completes.

  In general this is unnecessary, but should be safe as long as the chain
  only goes back one level - if the `IncrementalDriver` doesn't retain any
  previous `TrackedIncrementalBuildStrategy` instances. However, this does
  happen:

* `NgCompiler` indirectly causes retention of previous `NgCompiler`
  instances (and thus previous `TrackedIncrementalBuildStrategy` instances)
  through accidental capture of the `this` context in a closure created in
  its constructor. This closure is wrapped in a `ts.ModuleResolutionCache`
  used to create a `ModuleResolver` class, which is passed to the program's
  `TraitCompiler` on construction.

* The `IncrementalDriver` retains a reference to the `TraitCompiler` of the
  previous compilation, completing the reference chain.

The final retention chain thus looks like:

* `TrackedIncrementalBuildStrategy` of current program
* `.previous`: `IncrementalDriver` of previous program
* `.lastGood.traitCompiler`: `TraitCompiler`
* `.handlers[..].moduleResolver.moduleResolutionCache`: cache
* (via `getCanonicalFileName` closure): `NgCompiler`
* `.incrementalStrategy`: `TrackedIncrementalBuildStrategy` of previous
  program.

The closure link is the "real" leak here. `NgCompiler` is creating a closure
for `getCanonicalFileName`, delegating to its
`this.adapter.getCanonicalFileName`, for the purposes of creating a
`ts.ModuleResolutionCache`. The fact that the closure references
`NgCompiler` thus eventually causes previous `NgCompiler` iterations to be
retained. This is also potentially problematic due to the shared nature of
`ts.ModuleResolutionCache`, which is potentially retained across multiple
compilations intentionally.

This commit fixes the first two links in the retention chain: the build
strategy is patched to not retain a `previous` pointer, and the `NgCompiler`
is patched to not create a closure in the first place, but instead pass a
bound function. This ensures that the `NgCompiler` does not retain previous
instances of itself in the first place, even if the build strategy does
end up retaining the previous incremental state unnecessarily.

The third link (`IncrementalDriver` unnecessarily retaining the whole
`TraitCompiler`) is not addressed in this commit as it's a more
architectural problem that will require some refactoring. However, the leak
potential of this retention is eliminated thanks to fixing the first two
issues.

PR Close #37835

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

@@ -116,7 +116,13 @@ export class NgCompiler {
 
     const moduleResolutionCache = ts.createModuleResolutionCache(
         this.adapter.getCurrentDirectory(),
-        fileName => this.adapter.getCanonicalFileName(fileName));
+        // Note: this used to be an arrow-function closure. However, JS engines like v8 have some
+        // strange behaviors with retaining the lexical scope of the closure. Even if this function
+        // doesn't retain a reference to `this`, if other closures in the constructor here reference
+        // `this` internally then a closure created here would retain them. This can cause major
+        // memory leak issues since the `moduleResolutionCache` is a long-lived object and finds its
+        // way into all kinds of places inside TS internal objects.
+        this.adapter.getCanonicalFileName.bind(this.adapter));
     this.moduleResolver =
         new ModuleResolver(tsProgram, this.options, this.adapter, moduleResolutionCache);
     this.resourceManager = new AdapterResourceLoader(adapter, this.options);

packages/compiler-cli/src/ngtsc/incremental/src/strategy.ts

@@ -42,20 +42,22 @@ export class NoopIncrementalBuildStrategy implements IncrementalBuildStrategy {
  * Tracks an `IncrementalDriver` within the strategy itself.
  */
 export class TrackedIncrementalBuildStrategy implements IncrementalBuildStrategy {
-  private previous: IncrementalDriver|null = null;
+  private driver: IncrementalDriver|null = null;
-  private next: IncrementalDriver|null = null;
+  private isSet: boolean = false;
 
   getIncrementalDriver(): IncrementalDriver|null {
-    return this.next !== null ? this.next : this.previous;
+    return this.driver;
   }
 
   setIncrementalDriver(driver: IncrementalDriver): void {
-    this.next = driver;
+    this.driver = driver;
+    this.isSet = true;
   }
 
   toNextBuildStrategy(): TrackedIncrementalBuildStrategy {
     const strategy = new TrackedIncrementalBuildStrategy();
-    strategy.previous = this.next;
+    // Only reuse a driver that was explicitly set via `setIncrementalDriver`.
+    strategy.driver = this.isSet ? this.driver : null;
     return strategy;
   }
 }