When FileSystemState.changeFile() release data from unlinkedUnitStore.

This fixes b/269188397 in Cider because now unlikned data is present
in both ByteStore and UnlinkedUnitStore, or absent in both. Before
this change we would get it from UnlinkedUnitStore, but expected
that these bytes were also requested from ByteStore, so we attempted
to release them (and got "underflow" of reference counter).

Also, we now put the unlinked data into UnlinkedUnitStore not only
after reading bytes, but also after building it. So, we second use
will get it from the store, not the third.

Change-Id: I89bf554511d3be82f67ef1f237061ba8a4693592
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284745
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>

Author: scheglov

pkg/analyzer/lib/src/dart/analysis/byte_store.dart

@@ -42,6 +42,10 @@ class MemoryByteStore implements ByteStore {
   @visibleForTesting
   final Map<String, MemoryByteStoreEntry> map = {};
 
+  /// Throws [StateError] if [release] invoked when there is no entry.
+  @visibleForTesting
+  bool throwIfReleaseWithoutEntry = false;
+
   @override
   Uint8List? get(String key) {
     final entry = map[key];
@@ -74,6 +78,8 @@ class MemoryByteStore implements ByteStore {
         if (entry.refCount == 0) {
           map.remove(key);
         }
+      } else if (throwIfReleaseWithoutEntry) {
+        throw StateError('No entry: $key');
       }
     }
   }

pkg/analyzer/lib/src/dart/analysis/file_state.dart

@@ -690,6 +690,7 @@ class FileState {
       var bytes = driverUnlinkedUnit.toBytes();
       _fsState._byteStore.putGet(_unlinkedKey!, bytes);
       testData?.unlinkedKeyPut.add(unlinkedKey);
+      _fsState.unlinkedUnitStore.put(_unlinkedKey!, driverUnlinkedUnit);
       return driverUnlinkedUnit;
     });
   }
@@ -1196,6 +1197,9 @@ class FileSystemState {
     // The removed file does not reference other files anymore.
     file._kind?.dispose();
 
+    // Release this unlinked data.
+    unlinkedUnitStore.release(file.unlinkedKey);
+
     // Recursively remove files that reference the removed file.
     for (var reference in file.referencingFiles.toList()) {
       changeFile(reference.path, removedFiles);

pkg/analyzer/lib/src/dart/analysis/unlinked_unit_store.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analyzer/src/dart/analysis/unlinked_data.dart';
+import 'package:meta/meta.dart';
 
 abstract class UnlinkedUnitStore {
   void clear();
@@ -14,16 +15,17 @@ abstract class UnlinkedUnitStore {
 class UnlinkedUnitStoreImpl implements UnlinkedUnitStore {
   // TODO(jensj): Could we use finalizers and automatically clean up
   // this map?
-  final Map<String, _UnlinkedUnitStoreData> _deserializedUnlinked = {};
+  @visibleForTesting
+  final Map<String, _UnlinkedUnitStoreData> map = {};
 
   @override
   void clear() {
-    _deserializedUnlinked.clear();
+    map.clear();
   }
 
   @override
   AnalysisDriverUnlinkedUnit? get(String key) {
-    var lookup = _deserializedUnlinked[key];
+    var lookup = map[key];
     if (lookup != null) {
       lookup.usageCount++;
       return lookup.value.target;
@@ -33,16 +35,16 @@ class UnlinkedUnitStoreImpl implements UnlinkedUnitStore {
 
   @override
   void put(String key, AnalysisDriverUnlinkedUnit value) {
-    _deserializedUnlinked[key] = _UnlinkedUnitStoreData(WeakReference(value));
+    map[key] = _UnlinkedUnitStoreData(WeakReference(value));
   }
 
   @override
   void release(String key) {
-    var lookup = _deserializedUnlinked[key];
+    var lookup = map[key];
     if (lookup != null) {
       lookup.usageCount--;
       if (lookup.usageCount <= 0) {
-        _deserializedUnlinked.remove(key);
+        map.remove(key);
       }
     }
   }

pkg/analyzer/test/src/dart/analysis/analyzer_state_printer.dart

@@ -8,13 +8,15 @@ import 'package:analyzer/src/dart/analysis/driver.dart';
 import 'package:analyzer/src/dart/analysis/file_state.dart';
 import 'package:analyzer/src/dart/analysis/library_context.dart';
 import 'package:analyzer/src/dart/analysis/library_graph.dart';
+import 'package:analyzer/src/dart/analysis/unlinked_unit_store.dart';
 import 'package:analyzer/src/dart/micro/resolve_file.dart';
 import 'package:collection/collection.dart';
 import 'package:path/path.dart';
 import 'package:test/test.dart';
 
 class AnalyzerStatePrinter {
   final MemoryByteStore byteStore;
+  final UnlinkedUnitStoreImpl unlinkedUnitStore;
   final IdProvider idProvider;
   final LibraryContext libraryContext;
   final bool omitSdkFiles;
@@ -27,6 +29,7 @@ class AnalyzerStatePrinter {
 
   AnalyzerStatePrinter({
     required this.byteStore,
+    required this.unlinkedUnitStore,
     required this.idProvider,
     required this.libraryContext,
     required this.omitSdkFiles,
@@ -47,6 +50,7 @@ class AnalyzerStatePrinter {
     _writeFiles(testData.fileSystem);
     _writeLibraryContext(testData.libraryContext);
     _writeElementFactory();
+    _writeUnlinkedUnitStore();
     _writeByteStore();
   }
 
@@ -595,6 +599,21 @@ class AnalyzerStatePrinter {
     }
   }
 
+  void _writeUnlinkedUnitStore() {
+    _writelnWithIndent('unlinkedUnitStore');
+    _withIndent(() {
+      final groups = unlinkedUnitStore.map.entries.groupListsBy((element) {
+        return element.value.usageCount;
+      });
+
+      for (final groupEntry in groups.entries) {
+        final keys = groupEntry.value.map((e) => e.key).toList();
+        final shortKeys = idProvider.shortKeys(keys)..sort();
+        _writelnWithIndent('${groupEntry.key}: $shortKeys');
+      }
+    });
+  }
+
   void _writeUriList(String name, Iterable<Uri> uriIterable) {
     final uriStrList = <String>[];
     for (final uri in uriIterable) {

pkg/analyzer/test/src/dart/micro/file_resolution.dart

@@ -10,6 +10,7 @@ import 'package:analyzer/src/dart/analysis/byte_store.dart';
 import 'package:analyzer/src/dart/analysis/file_state.dart';
 import 'package:analyzer/src/dart/analysis/library_context.dart';
 import 'package:analyzer/src/dart/analysis/performance_logger.dart';
+import 'package:analyzer/src/dart/analysis/unlinked_unit_store.dart';
 import 'package:analyzer/src/dart/micro/resolve_file.dart';
 import 'package:analyzer/src/dart/sdk/sdk.dart';
 import 'package:analyzer/src/test_utilities/find_element.dart';
@@ -69,6 +70,7 @@ class FileResolutionTest with ResourceProviderMixin, ResolutionTest {
     final buffer = StringBuffer();
     printer.AnalyzerStatePrinter(
       byteStore: byteStore,
+      unlinkedUnitStore: fsState.unlinkedUnitStore as UnlinkedUnitStoreImpl,
       idProvider: _idProvider,
       libraryContext: libraryContext,
       omitSdkFiles: omitSdkFiles,

pkg/analyzer/test/src/dart/micro/simple_file_resolver_test.dart

@@ -116,6 +116,8 @@ elementFactory
     package:dart.test/a.dart
     package:dart.test/b.dart
     package:dart.test/c.dart
+unlinkedUnitStore
+  1: [k00, k01, k02, k06, k07, k08, k09, k10]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08, k09, k10, k11]
 ''');
@@ -193,6 +195,8 @@ elementFactory
     package:dart.test/a.dart
     package:dart.test/b.dart
     package:dart.test/c.dart
+unlinkedUnitStore
+  1: [k00, k01, k02, k06, k07, k08, k09, k10]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08, k09, k10, k11]
 ''');
@@ -270,6 +274,8 @@ elementFactory
     package:dart.test/a.dart
     package:dart.test/b.dart
     package:dart.test/c.dart
+unlinkedUnitStore
+  1: [k00, k01, k02, k06, k07, k08, k09, k10]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08, k09, k10, k11]
 ''');
@@ -315,6 +321,8 @@ libraryCycles
 elementFactory
   hasElement
     package:dart.test/b.dart
+unlinkedUnitStore
+  1: [k01, k06, k07, k08, k09, k10]
 byteStore
   1: [k01, k04, k06, k07, k08, k09, k10, k11]
 ''');
@@ -392,6 +400,8 @@ elementFactory
     package:dart.test/a.dart
     package:dart.test/b.dart
     package:dart.test/c.dart
+unlinkedUnitStore
+  1: [k00, k01, k02, k06, k07, k08, k09, k10]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08, k09, k10, k11]
 ''');
@@ -553,6 +563,8 @@ libraryCycles
 elementFactory
   hasElement
     package:dart.test/a.dart
+unlinkedUnitStore
+  1: [k00, k01, k03, k04, k05, k06, k07]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08]
 ''');
@@ -574,6 +586,8 @@ libraryCycles
     get: []
     put: [k02]
 elementFactory
+unlinkedUnitStore
+  1: [k03, k04, k05, k06, k07]
 byteStore
   1: [k03, k04, k05, k06, k07, k08]
 ''');
@@ -617,6 +631,8 @@ libraryCycles
 elementFactory
   hasElement
     package:dart.test/a.dart
+unlinkedUnitStore
+  1: [k00, k01, k03, k04, k05, k06, k07]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08]
 ''');
@@ -700,6 +716,8 @@ elementFactory
   hasElement
     package:dart.test/a.dart
     package:dart.test/c.dart
+unlinkedUnitStore
+  1: [k00, k01, k02, k05, k06, k07, k08, k09]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08, k09, k10]
 ''');
@@ -728,6 +746,8 @@ libraryCycles
     get: []
     put: [k04]
 elementFactory
+unlinkedUnitStore
+  1: [k05, k06, k07, k08, k09]
 byteStore
   1: [k05, k06, k07, k08, k09, k10]
 ''');
@@ -794,6 +814,8 @@ elementFactory
   hasElement
     package:dart.test/a.dart
     package:dart.test/c.dart
+unlinkedUnitStore
+  1: [k00, k01, k02, k05, k06, k07, k08, k09]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08, k09, k10]
 ''');
@@ -942,6 +964,8 @@ libraryCycles
 elementFactory
   hasElement
     package:dart.test/a.dart
+unlinkedUnitStore
+  1: [k00, k02, k03, k04, k05, k06]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07]
 ''');
@@ -961,6 +985,8 @@ libraryCycles
     get: []
     put: [k01]
 elementFactory
+unlinkedUnitStore
+  1: [k00, k02, k03, k04, k05, k06]
 byteStore
 ''');
   }
@@ -1511,6 +1537,8 @@ libraryCycles
 elementFactory
   hasReader
     package:dart.test/a.dart
+unlinkedUnitStore
+  1: [k00, k02, k03, k04, k05, k06]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07]
 ''');
@@ -1545,6 +1573,8 @@ elementFactory
     package:dart.test/a.dart
   hasReader
     package:dart.test/a.dart
+unlinkedUnitStore
+  1: [k00, k02, k03, k04, k05, k06]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07]
 ''');
@@ -1602,6 +1632,8 @@ elementFactory
   hasReader
     package:dart.test/a.dart
     package:dart.test/b.dart
+unlinkedUnitStore
+  1: [k00, k02, k03, k04, k05, k06, k08]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08, k09]
 ''');
@@ -1668,6 +1700,8 @@ elementFactory
   hasReader
     package:dart.test/a.dart
     package:dart.test/b.dart
+unlinkedUnitStore
+  1: [k00, k02, k03, k04, k05, k06, k08]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08, k09]
 ''');
@@ -1711,6 +1745,8 @@ libraryCycles
 elementFactory
   hasReader
     package:dart.test/test.dart
+unlinkedUnitStore
+  1: [k00, k02, k03, k04, k05, k06]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07]
 ''');
@@ -1751,6 +1787,8 @@ elementFactory
     package:dart.test/test.dart
   hasReader
     package:dart.test/test.dart
+unlinkedUnitStore
+  1: [k00, k02, k03, k04, k05, k06]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07]
 ''');
@@ -1936,6 +1974,8 @@ elementFactory
     package:dart.aaa/a.dart
     package:dart.aaa/b.dart
     package:dart.aaa/c.dart
+unlinkedUnitStore
+  1: [k00, k01, k02, k06, k07, k08, k09, k10]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08, k09, k10, k11]
 ''');
@@ -1998,6 +2038,8 @@ elementFactory
   hasElement
     package:dart.aaa/a.dart
     package:dart.aaa/c.dart
+unlinkedUnitStore
+  1: [k00, k02, k06, k07, k08, k09, k10]
 byteStore
   1: [k00, k02, k03, k05, k06, k07, k08, k09, k10, k11]
 ''');
@@ -2167,6 +2209,8 @@ elementFactory
     package:dart.aaa/d.dart
     package:dart.aaa/e.dart
     package:dart.aaa/f.dart
+unlinkedUnitStore
+  1: [k00, k01, k02, k03, k04, k05, k12, k13, k14, k15, k16]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07, k08, k09, k10, k11, k12, k13, k14, k15, k16, k17]
 ''');
@@ -2278,6 +2322,8 @@ elementFactory
     package:dart.aaa/c.dart
     package:dart.aaa/d.dart
     package:dart.aaa/f.dart
+unlinkedUnitStore
+  1: [k00, k02, k03, k05, k12, k13, k14, k15, k16]
 byteStore
   1: [k00, k02, k03, k05, k06, k08, k09, k11, k12, k13, k14, k15, k16, k17]
 ''');
@@ -2319,6 +2365,8 @@ libraryCycles
 elementFactory
   hasElement
     package:dart.aaa/a.dart
+unlinkedUnitStore
+  1: [k00, k02, k03, k04, k05, k06]
 byteStore
   1: [k00, k01, k02, k03, k04, k05, k06, k07]
 ''');

pkg/analyzer/test/src/dart/resolution/context_collection_resolution.dart

@@ -13,6 +13,7 @@ import 'package:analyzer/src/dart/analysis/byte_store.dart';
 import 'package:analyzer/src/dart/analysis/driver.dart';
 import 'package:analyzer/src/dart/analysis/driver_based_analysis_context.dart';
 import 'package:analyzer/src/dart/analysis/experiments.dart';
+import 'package:analyzer/src/dart/analysis/unlinked_unit_store.dart';
 import 'package:analyzer/src/generated/engine.dart' show AnalysisOptionsImpl;
 import 'package:analyzer/src/generated/sdk.dart';
 import 'package:analyzer/src/summary2/kernel_compilation_service.dart';
@@ -180,6 +181,8 @@ abstract class ContextResolutionTest
     final buffer = StringBuffer();
     AnalyzerStatePrinter(
       byteStore: _byteStore,
+      unlinkedUnitStore:
+          analysisDriver.fsState.unlinkedUnitStore as UnlinkedUnitStoreImpl,
       idProvider: _idProvider,
       libraryContext: analysisDriver.libraryContext,
       omitSdkFiles: omitSdkFiles,