Fix an issue where multiple AnalyzerResolvers instances lead to incorrect behavior

build/CHANGELOG.md

@@ -1,3 +1,5 @@
+## 2.4.2-wip
+
 ## 2.4.1
 
 - Allow the latest analyzer (6.x.x).

build/pubspec.yaml

@@ -1,5 +1,5 @@
 name: build
-version: 2.4.1
+version: 2.4.2-wip
 description: A package for authoring build_runner compatible code generators.
 repository: https://github.com/dart-lang/build/tree/master/build
 
@@ -18,7 +18,7 @@ dependencies:
   path: ^1.8.0
 
 dev_dependencies:
-  build_resolvers: ^2.0.0
+  build_resolvers: ^2.3.3
   build_test: ^2.0.0
   dart_flutter_team_lints: ^1.0.0
   test: ^1.16.0

build/test/builder/build_step_impl_test.dart

@@ -35,7 +35,7 @@ void main() {
       primary = makeAssetId();
       outputs = List.generate(5, (index) => makeAssetId());
       buildStep = BuildStepImpl(primary, outputs, reader, writer,
-          AnalyzerResolvers(), resourceManager, _unsupported);
+          AnalyzerResolvers.custom(), resourceManager, _unsupported);
     });
 
     test('doesnt allow non-expected outputs', () {
@@ -93,7 +93,7 @@ void main() {
         [outputId],
         reader,
         writer,
-        AnalyzerResolvers(),
+        AnalyzerResolvers.custom(),
         resourceManager,
         _unsupported,
       );
@@ -121,7 +121,7 @@ void main() {
 
         var primary = makeAssetId('a|web/a.dart');
         var buildStep = BuildStepImpl(primary, [], reader, writer,
-            AnalyzerResolvers(), resourceManager, _unsupported);
+            AnalyzerResolvers.custom(), resourceManager, _unsupported);
         var resolver = buildStep.resolver;
 
         var aLib = await resolver.libraryFor(primary);
@@ -150,8 +150,14 @@ void main() {
       assetWriter = SlowAssetWriter();
       outputId = makeAssetId('a|test.txt');
       outputContent = '$outputId';
-      buildStep = BuildStepImpl(primary, [outputId], StubAssetReader(),
-          assetWriter, AnalyzerResolvers(), resourceManager, _unsupported);
+      buildStep = BuildStepImpl(
+          primary,
+          [outputId],
+          StubAssetReader(),
+          assetWriter,
+          AnalyzerResolvers.custom(),
+          resourceManager,
+          _unsupported);
     });
 
     test('Completes only after writes finish', () async {
@@ -199,7 +205,7 @@ void main() {
       primary = makeAssetId();
       output = makeAssetId();
       buildStep = BuildStepImpl(primary, [output], reader, writer,
-          AnalyzerResolvers(), resourceManager, _unsupported,
+          AnalyzerResolvers.custom(), resourceManager, _unsupported,
           stageTracker: NoOpStageTracker.instance);
     });
 
@@ -214,7 +220,7 @@ void main() {
     var writer = StubAssetWriter();
     var unused = <AssetId>{};
     var buildStep = BuildStepImpl(makeAssetId(), [], reader, writer,
-        AnalyzerResolvers(), resourceManager, _unsupported,
+        AnalyzerResolvers.custom(), resourceManager, _unsupported,
         reportUnusedAssets: unused.addAll);
     var reported = [
       makeAssetId(),

build_resolvers/CHANGELOG.md

@@ -1,3 +1,11 @@
+## 2.3.3-wip
+
+- Deprecate the unnamed `AnalyzerResolvers` constructor, an replace it with the
+  `AnalyzerResolvers.sharedInstance` getter and `AnalyzerResolvers.custom`
+  constructor. These new apis enforce a 1:1 relationship between the
+  `BuildAssetUriResolver` instances and `AnalyzerResolvers` instances, which is
+  necessary to ensure correct builds in the presence of multiple resolvers.
+
 ## 2.3.2
 
 - Skip file delete for SDK summary and deps file. This will only impact behavior

build_resolvers/lib/src/build_asset_uri_resolver.dart

@@ -54,10 +54,16 @@ class BuildAssetUriResolver extends UriResolver {
   /// input, subsequent calls to a resolver, or a transitive import thereof.
   final _buildStepTransitivelyResolvedAssets = <BuildStep, HashSet<AssetId>>{};
 
-  /// Use the [instance] getter to get an instance.
-  BuildAssetUriResolver._();
+  /// Use the [sharedInstance] getter to get the shared instance, which is what
+  /// real builds should do. There should always be a 1:1 relationship between
+  /// [BuildAssetUriResolver]s and `AnalyzerResolvers`.
+  BuildAssetUriResolver();
 
-  static final BuildAssetUriResolver instance = BuildAssetUriResolver._();
+  /// The instance used by the shared `AnalyzerResolvers` instance, which is
+  /// what should be used by normal builds.
+  ///
+  /// This is not used within testing contexts or similar custom contexts.
+  static final BuildAssetUriResolver sharedInstance = BuildAssetUriResolver();
 
   /// Updates [resourceProvider] and the analysis driver given by
   /// `withDriverResource`  with updated versions of [entryPoints].
@@ -261,7 +267,7 @@ Set<AssetId> _parseDependencies(String content, AssetId from) => HashSet.of(
 /// directives, and cache the results if they weren't already cached.
 Future<Iterable<AssetId>?> dependenciesOf(
         AssetId id, BuildStep buildStep) async =>
-    (await BuildAssetUriResolver.instance
+    (await BuildAssetUriResolver.sharedInstance
             ._updateCachedAssetState(id, buildStep))
         ?.dependencies;
 

build_resolvers/lib/src/resolver.dart

@@ -381,7 +381,8 @@ class AnalyzerResolvers implements Resolvers {
 
   // Lazy, all access must be preceded by a call to `_ensureInitialized`.
   late final AnalyzerResolver _resolver;
-  BuildAssetUriResolver? _uriResolver;
+
+  final BuildAssetUriResolver _uriResolver;
 
   /// Nullable, should not be accessed outside of [_ensureInitialized].
   Future<Result<void>>? _initialized;
@@ -402,30 +403,65 @@ class AnalyzerResolvers implements Resolvers {
   /// **NOTE**: The [_packageConfig] is not used for path resolution, it is
   /// primarily used to get the language versions. Any other data (including
   /// extra data), may be passed to the analyzer on an as needed basis.
-  AnalyzerResolvers(
-      [AnalysisOptions? analysisOptions,
-      Future<String> Function()? sdkSummaryGenerator,
-      this._packageConfig])
-      : _analysisOptions = analysisOptions ??
+  factory AnalyzerResolvers.custom({
+    AnalysisOptions? analysisOptions,
+    Future<String> Function()? sdkSummaryGenerator,
+    PackageConfig? packageConfig,
+  }) =>
+      AnalyzerResolvers._(
+          analysisOptions: analysisOptions,
+          sdkSummaryGenerator: sdkSummaryGenerator,
+          packageConfig: packageConfig,
+          // Custom resolvers get their own asset uri resolver, as there should
+          // always be a 1:1 relationship between them.
+          uriResolver: BuildAssetUriResolver());
+
+  /// See [AnalyzerResolvers.custom] for docs.
+  @Deprecated('Use either the AnalyzerResolvers.custom constructor or the '
+      'AnalyzerResolvers.sharedInstance static getter to get an instance.')
+  factory AnalyzerResolvers([
+    AnalysisOptions? analysisOptions,
+    Future<String> Function()? sdkSummaryGenerator,
+    PackageConfig? packageConfig,
+  ]) =>
+      AnalyzerResolvers._(
+          analysisOptions: analysisOptions,
+          sdkSummaryGenerator: sdkSummaryGenerator,
+          packageConfig: packageConfig,
+          // For backwards compatibility we use the shared instance here.
+          uriResolver: BuildAssetUriResolver.sharedInstance);
+
+  /// See [AnalyzerResolvers.custom] for docs.
+  AnalyzerResolvers._({
+    AnalysisOptions? analysisOptions,
+    PackageConfig? packageConfig,
+    Future<String> Function()? sdkSummaryGenerator,
+    required BuildAssetUriResolver uriResolver,
+  })  : _analysisOptions = analysisOptions ??
             (AnalysisOptionsImpl()
               ..contextFeatures =
                   _featureSet(enableExperiments: enabledExperiments)),
+        _packageConfig = packageConfig,
         _sdkSummaryGenerator =
-            sdkSummaryGenerator ?? defaultSdkSummaryGenerator;
+            sdkSummaryGenerator ?? defaultSdkSummaryGenerator,
+        _uriResolver = uriResolver;
+
+  /// The instance that most real build systems should use.
+  static final AnalyzerResolvers sharedInstance =
+      AnalyzerResolvers._(uriResolver: BuildAssetUriResolver.sharedInstance);
 
   /// Create a Resolvers backed by an `AnalysisContext` using options
   /// [_analysisOptions].
   Future<void> _ensureInitialized() {
     return Result.release(_initialized ??= Result.capture(() async {
       _warnOnLanguageVersionMismatch();
-      final uriResolver = _uriResolver = BuildAssetUriResolver.instance;
       final loadedConfig = _packageConfig ??=
           await loadPackageConfigUri((await Isolate.packageConfig)!);
-      var driver = await analysisDriver(uriResolver, _analysisOptions,
+      var driver = await analysisDriver(_uriResolver, _analysisOptions,
           await _sdkSummaryGenerator(), loadedConfig);
 
-      _resolver =
-          AnalyzerResolver(driver, _driverPool, _readAndWritePool, uriResolver);
+      _resolver = AnalyzerResolver(
+          driver, _driverPool, _readAndWritePool, _uriResolver);
     }()));
   }
 
@@ -439,7 +475,7 @@ class AnalyzerResolvers implements Resolvers {
   /// Must be called between each build.
   @override
   void reset() {
-    _uriResolver?.reset();
+    _uriResolver.reset();
   }
 }
 

build_resolvers/pubspec.yaml

@@ -1,5 +1,5 @@
 name: build_resolvers
-version: 2.3.2
+version: 2.3.3-wip
 description: Resolve Dart code in a Builder
 repository: https://github.com/dart-lang/build/tree/master/build_resolvers
 

build_runner_core/CHANGELOG.md

@@ -1,7 +1,9 @@
-## 7.2.11-dev
+## 7.2.11-wip
 
 - Add better error messages for incomplete package configs when creating a
   package graph.
+- Update to build_resolvers 2.3.3 and use the shared analyzer resolvers
+  instance.
 
 ## 7.2.10
 

build_runner_core/lib/src/generate/options.dart

@@ -208,7 +208,7 @@ feature, you may need to run `dart run build_runner clean` and then rebuild.
       }
       trackPerformance = true;
     }
-    resolvers ??= AnalyzerResolvers();
+    resolvers ??= AnalyzerResolvers.sharedInstance;
 
     return BuildOptions._(
       debounceDelay: debounceDelay,

build_runner_core/pubspec.yaml

@@ -1,5 +1,5 @@
 name: build_runner_core
-version: 7.2.11-dev
+version: 7.2.11-wip
 description: Core tools to organize the structure of a build and run Builders.
 repository: https://github.com/dart-lang/build/tree/master/build_runner_core
 
@@ -15,7 +15,7 @@ dependencies:
   async: ^2.5.0
   build: ^2.4.0
   build_config: ^1.0.0
-  build_resolvers: ^2.0.0
+  build_resolvers: ^2.3.3
   collection: ^1.15.0
   convert: ^3.0.0
   crypto: ^3.0.0

build_test/CHANGELOG.md

@@ -1,6 +1,8 @@
 ## 2.2.1-dev
 
 - Avoid passing a nullable value to `Future.value`.
+- Update to build_resolvers 2.3.3, which resolves some race conditions when
+  using multiple resolvers instances.
 
 ## 2.2.0
 

build_test/lib/src/resolve_source.dart

@@ -15,8 +15,6 @@ import 'in_memory_writer.dart';
 import 'multi_asset_reader.dart';
 import 'package_reader.dart';
 
-final defaultResolvers = AnalyzerResolvers();
-
 /// Marker constant that may be used in combination with [resolveSources].
 ///
 /// Use of this string means instead of using the contents of the string as the
@@ -199,8 +197,8 @@ Future<T> _resolveAssets<T>(
   // Use the default resolver if no experiments are enabled. This is much
   // faster.
   resolvers ??= packageConfig == null && enabledExperiments.isEmpty
-      ? defaultResolvers
-      : AnalyzerResolvers(null, null, resolvedConfig);
+      ? AnalyzerResolvers.sharedInstance
+      : AnalyzerResolvers.custom(packageConfig: resolvedConfig);
 
   // We don't care about the results of this build, but we also can't await
   // it because that would block on the `tearDown` of the `resolveBuilder`.

build_test/lib/src/test_builder.dart

@@ -14,7 +14,6 @@ import 'assets.dart';
 import 'in_memory_reader.dart';
 import 'in_memory_writer.dart';
 import 'multi_asset_reader.dart';
-import 'resolve_source.dart';
 import 'written_asset_reader.dart';
 
 AssetId _passThrough(AssetId id) => id;
@@ -171,8 +170,8 @@ Future testBuilder(
   var logger = Logger('testBuilder');
   var logSubscription = logger.onRecord.listen(onLog);
   var resolvers = packageConfig == null && enabledExperiments.isEmpty
-      ? defaultResolvers
-      : AnalyzerResolvers(null, null, packageConfig);
+      ? AnalyzerResolvers.sharedInstance
+      : AnalyzerResolvers.custom(packageConfig: packageConfig);
 
   for (var input in inputIds) {
     // create another writer spy and reader for each input. This prevents writes

build_test/pubspec.yaml

@@ -10,7 +10,7 @@ dependencies:
   async: ^2.5.0
   build: ^2.1.0
   build_config: ^1.0.0
-  build_resolvers: ^2.0.0
+  build_resolvers: ^2.3.3
   crypto: ^3.0.0
   glob: ^2.0.0
   html: ^0.15.0