[dartdevc] don't clear generic type caches on a hot restart

vsmenon · commit-bot@chromium.org · commit 3605630e8abf · 2019-07-23T13:54:31.000Z
The combination of type expression hoisting, hot restart, and generic cache reset triggers occasional breakage (see #37259). Not clearing the generic cache should be safe, but will some leak memory: unloaded types will pollute the cache. References to those unloaded types in user code, however, should be cleared. Bug: #37259 Change-Id: Ia15115a41556db7a19109f0178baa63ec0cfcb9c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109100 Reviewed-by: Leaf Petersen <leafp@google.com> Commit-Queue: Vijay Menon <vsm@google.com>
diff --git a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/classes.dart b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/classes.dart
@@ -127,7 +127,14 @@ generic(typeConstructor, setBaseClass) => JS('', '''(() => {
     $throwInternalError('must have at least one generic type argument');
   }
   let resultMap = new Map();
-  $_cacheMaps.push(resultMap);
+  // TODO(vsm): Rethink how to clear the resultMap on hot restart.
+  // A simple clear via:
+  //   _cacheMaps.push(resultMap);
+  // will break (a) we hoist type expressions in generated code and
+  // (b) we don't clear those type expressions in the presence of a
+  // hot restart.  Not clearing this map (as we're doing now) should
+  // not affect correctness, but can result in a memory leak across
+  // multiple restarts.
   function makeGenericType(...args) {
     if (args.length != length && args.length != 0) {
       $throwInternalError('requires ' + length + ' or 0 type arguments');
diff --git a/tests/compiler/dartdevc_native/hot_restart_test.dart b/tests/compiler/dartdevc_native/hot_restart_test.dart
@@ -0,0 +1,34 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'dart:_foreign_helper' show JS;
+import 'dart:_runtime' as dart;
+
+class Foo<T> {
+  Type type() => typeOf<Foo<T>>();
+}
+
+class Bar {}
+
+Type typeOf<T>() => T;
+
+Type fooOf<T>() => typeOf<Foo<T>>();
+
+void main() {
+  var f1 = Foo<Bar>();
+  var t1 = typeOf<Foo<Bar>>();
+  Expect.equals(f1.type(), t1);
+  var s1 = fooOf<Bar>();
+  Expect.equals(t1, s1);
+
+  dart.hotRestart();
+
+  var f2 = Foo<Bar>();
+  Expect.isTrue(f2 is Foo<Bar>);
+  var t2 = typeOf<Foo<Bar>>();
+  Expect.equals(f2.type(), t2);
+  var s2 = fooOf<Bar>();
+  Expect.equals(t2, s2);
+}
