[frontend_server] Plug leaks caused by saving the first compilation r…

…esult

The first compilation result is leaked in two ways:
1) Directly by saving the component in a variable; and
2) Via an unfortunate context thing, probably a variation of
   http://dartbug.com/36983. I will update that bug with a reproduction
  example later.

The reason this creates a (big) leak is illustrated with an example:
* Say the first component (A) has 10 libraries in it. Each of these
  libraries has parent pointers and points (currently) to A, which again
  points to all of the 10 libraries.
* We then do a recompilation, say 5 libraries are reused and 5 are new.
  They are put into a component (B). We really should have 10 libraries,
  the 5 old ones and the 5 new ones (and for simplicity lets say these are
  the ones in B). Notice that the 5 old ones will have their parent
  pointers updated and also still be in the list of libraries in A.
  We keep 15 libraries alive because we have the 10 original ones saved via
  A and the 10 (where 5 is new) saved via B.
* We then do a recompilation, say 2 of the same libraries as was also
  recompiled before, these end up in compnent C which has 5 libraries from A,
  3 libraries from B and the 2 new ones. All of these libraries will have
  their parent pointers updated to point to C.
* Because we saved A we keep all the 10 libraries in A though.
  Because we saved A and some of the libraries in A had parent pointers
  updated to point to B we also keep B and all libraries in B.
  Because we keep B and some of the libraries in B had parent pointers
  updated to point to C we also keep C and all libraries in C.
  So instead of only having the 10 "live" libraries, we have 10 + 5 + 2 = 17
  libraries, a leak of 7. With more compilations this keeps happening and
  the leak keeps growing.

This CL stops the leak by not holding on to A (which, in turn, stops holding
on to B etc.)

Change-Id: If4f8b1e240b7c39f084df9cb2690570ff26fa9b3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149280
Commit-Queue: Jens Johansen <[email protected]>
Reviewed-by: Vyacheslav Egorov <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

pkg/frontend_server/lib/frontend_server.dart

@@ -328,7 +328,6 @@ class FrontendCompiler implements CompilerInterface {
 
   IncrementalCompiler _generator;
   JavaScriptBundler _bundler;
-  Component _component;
 
   String _kernelBinaryFilename;
   String _kernelBinaryFilenameIncremental;
@@ -341,6 +340,30 @@ class FrontendCompiler implements CompilerInterface {
 
   final List<String> errors = List<String>();
 
+  _onDiagnostic(DiagnosticMessage message) {
+    bool printMessage;
+    switch (message.severity) {
+      case Severity.error:
+      case Severity.internalProblem:
+        printMessage = true;
+        errors.addAll(message.plainTextFormatted);
+        break;
+      case Severity.warning:
+        printMessage = true;
+        break;
+      case Severity.context:
+      case Severity.ignored:
+        throw 'Unexpected severity: ${message.severity}';
+    }
+    if (printMessage) {
+      printDiagnosticMessage(message, _outputStream.writeln);
+    }
+  }
+
+  void _installDartdevcTarget() {
+    targets['dartdevc'] = (TargetFlags flags) => DevCompilerTarget(flags);
+  }
+
   @override
   Future<bool> compile(
     String entryPoint,
@@ -378,25 +401,7 @@ class FrontendCompiler implements CompilerInterface {
           parseExperimentalArguments(options['enable-experiment']),
           onError: (msg) => errors.add(msg))
       ..nnbdMode = options['null-safety'] ? NnbdMode.Strong : NnbdMode.Weak
-      ..onDiagnostic = (DiagnosticMessage message) {
-        bool printMessage;
-        switch (message.severity) {
-          case Severity.error:
-          case Severity.internalProblem:
-            printMessage = true;
-            errors.addAll(message.plainTextFormatted);
-            break;
-          case Severity.warning:
-            printMessage = true;
-            break;
-          case Severity.context:
-          case Severity.ignored:
-            throw 'Unexpected severity: ${message.severity}';
-        }
-        if (printMessage) {
-          printDiagnosticMessage(message, _outputStream.writeln);
-        }
-      };
+      ..onDiagnostic = _onDiagnostic;
 
     if (options.wasParsed('libraries-spec')) {
       compilerOptions.librariesSpecificationUri =
@@ -447,7 +452,7 @@ class FrontendCompiler implements CompilerInterface {
     )..parseCommandLineFlags(options['bytecode-options']);
 
     // Initialize additional supported kernel targets.
-    targets['dartdevc'] = (TargetFlags flags) => DevCompilerTarget(flags);
+    _installDartdevcTarget();
     compilerOptions.target = createFrontEndTarget(
       options['target'],
       trackWidgetCreation: options['track-widget-creation'],
@@ -496,8 +501,6 @@ class FrontendCompiler implements CompilerInterface {
           component.uriToSource.keys);
 
       incrementalSerializer = _generator.incrementalSerializer;
-      _component = component;
-      _component.computeCanonicalNames();
     } else {
       if (options['link-platform']) {
         // TODO(aam): Remove linkedDependencies once platform is directly embedded
@@ -538,8 +541,10 @@ class FrontendCompiler implements CompilerInterface {
       }
 
       _kernelBinaryFilename = _kernelBinaryFilenameIncremental;
-    } else
+    } else {
       _outputStream.writeln(boundaryKey);
+    }
+    results = null; // Fix leak: Probably variation of http://dartbug.com/36983.
     return errors.isEmpty;
   }
 
@@ -917,8 +922,11 @@ class FrontendCompiler implements CompilerInterface {
       }
       assert(kernel2jsCompiler != null);
 
+      Component component = _generator.lastKnownGoodComponent;
+      component.computeCanonicalNames();
+
       var evaluator = new ExpressionCompiler(
-          _generator.generator, kernel2jsCompiler, _component,
+          _generator.generator, kernel2jsCompiler, component,
           verbose: _compilerOptions.verbose,
           onDiagnostic: _compilerOptions.onDiagnostic);
 

pkg/vm/lib/incremental_compiler.dart

@@ -34,6 +34,7 @@ class IncrementalCompiler {
 
   Uri get entryPoint => _entryPoint;
   IncrementalKernelGenerator get generator => _generator;
+  Component get lastKnownGoodComponent => _lastKnownGood;
 
   IncrementalCompiler(this._compilerOptions, this._entryPoint,
       {this.initializeFromDillUri, bool incrementalSerialization: true})