[native_assets_builder] Lock the build directory

pkgs/native_assets_builder/CHANGELOG.md

@@ -4,6 +4,8 @@
   `LinkConfig.dependencies` no longer have to specify Dart sources.
 - `DataAsset` test projects report all assets from `assets/` dir and default the
   asset names to the path inside the package.
+- Automatically locks build directories to prevent concurrency issues with
+  multiple concurrent `dart` and or `flutter` invocations.
 
 ## 0.8.1
 

pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart

@@ -6,6 +6,7 @@ import 'dart:async';
 import 'dart:io';
 
 import 'package:logging/logging.dart';
+import 'package:native_assets_cli/locking.dart';
 import 'package:native_assets_cli/native_assets_cli.dart' as api;
 import 'package:native_assets_cli/native_assets_cli_internal.dart';
 import 'package:package_config/package_config.dart';
@@ -34,11 +35,13 @@ typedef DependencyMetadata = Map<String, Metadata>;
 class NativeAssetsBuildRunner {
   final Logger logger;
   final Uri dartExecutable;
+  final Duration singleHookTimeout;
 
   NativeAssetsBuildRunner({
     required this.logger,
     required this.dartExecutable,
-  });
+    Duration? singleHookTimeout,
+  }) : singleHookTimeout = singleHookTimeout ?? const Duration(minutes: 5);
 
   /// [workingDirectory] is expected to contain `.dart_tool`.
   ///
@@ -413,14 +416,19 @@ class NativeAssetsBuildRunner {
         continue;
       }
       // TODO(https://github.com/dart-lang/native/issues/1321): Should dry runs be cached?
-      var (buildOutput, packageSuccess) = await _runHookForPackage(
-        hook,
-        config,
-        packageConfigUri,
-        workingDirectory,
-        includeParentEnvironment,
-        null,
-        hookKernelFile,
+      var (buildOutput, packageSuccess) = await runUnderDirectoryLock(
+        Directory.fromUri(config.outputDirectory.parent),
+        timeout: singleHookTimeout,
+        logger: logger,
+        () => _runHookForPackage(
+          hook,
+          config,
+          packageConfigUri,
+          workingDirectory,
+          includeParentEnvironment,
+          null,
+          hookKernelFile,
+        ),
       );
       buildOutput = _expandArchsNativeCodeAssets(buildOutput);
       hookResult = hookResult.copyAdd(buildOutput, packageSuccess);
@@ -460,47 +468,54 @@ class NativeAssetsBuildRunner {
     Uri? resources,
   ) async {
     final outDir = config.outputDirectory;
-    final (
-      compileSuccess,
-      hookKernelFile,
-      hookLastSourceChange,
-    ) = await _compileHookForPackageCached(
-      config,
-      packageConfigUri,
-      workingDirectory,
-      includeParentEnvironment,
-    );
-    if (!compileSuccess) {
-      return (HookOutputImpl(), false);
-    }
-
-    final hookOutput = HookOutputImpl.readFromFile(file: config.outputFile);
-    if (hookOutput != null) {
-      final lastBuilt = hookOutput.timestamp.roundDownToSeconds();
-      final dependenciesLastChange =
-          await hookOutput.dependenciesModel.lastModified();
-      if (lastBuilt.isAfter(dependenciesLastChange) &&
-          lastBuilt.isAfter(hookLastSourceChange)) {
-        logger.info(
-          'Skipping ${hook.name} for ${config.packageName} in $outDir. '
-          'Last build on $lastBuilt. '
-          'Last dependencies change on $dependenciesLastChange. '
-          'Last hook change on $hookLastSourceChange.',
+    return await runUnderDirectoryLock(
+      Directory.fromUri(config.outputDirectory.parent),
+      timeout: singleHookTimeout,
+      logger: logger,
+      () async {
+        final (
+          compileSuccess,
+          hookKernelFile,
+          hookLastSourceChange,
+        ) = await _compileHookForPackageCached(
+          config,
+          packageConfigUri,
+          workingDirectory,
+          includeParentEnvironment,
         );
-        // All build flags go into [outDir]. Therefore we do not have to check
-        // here whether the config is equal.
-        return (hookOutput, true);
-      }
-    }
+        if (!compileSuccess) {
+          return (HookOutputImpl(), false);
+        }
+
+        final hookOutput = HookOutputImpl.readFromFile(file: config.outputFile);
+        if (hookOutput != null) {
+          final lastBuilt = hookOutput.timestamp.roundDownToSeconds();
+          final dependenciesLastChange =
+              await hookOutput.dependenciesModel.lastModified();
+          if (lastBuilt.isAfter(dependenciesLastChange) &&
+              lastBuilt.isAfter(hookLastSourceChange)) {
+            logger.info(
+              'Skipping ${hook.name} for ${config.packageName} in $outDir. '
+              'Last build on $lastBuilt. '
+              'Last dependencies change on $dependenciesLastChange. '
+              'Last hook change on $hookLastSourceChange.',
+            );
+            // All build flags go into [outDir]. Therefore we do not have to
+            // check here whether the config is equal.
+            return (hookOutput, true);
+          }
+        }
 
-    return await _runHookForPackage(
-      hook,
-      config,
-      packageConfigUri,
-      workingDirectory,
-      includeParentEnvironment,
-      resources,
-      hookKernelFile,
+        return await _runHookForPackage(
+          hook,
+          config,
+          packageConfigUri,
+          workingDirectory,
+          includeParentEnvironment,
+          resources,
+          hookKernelFile,
+        );
+      },
     );
   }
 
@@ -849,3 +864,7 @@ extension on DateTime {
       DateTime.fromMillisecondsSinceEpoch(millisecondsSinceEpoch -
           millisecondsSinceEpoch % const Duration(seconds: 1).inMilliseconds);
 }
+
+extension on Uri {
+  Uri get parent => File(toFilePath()).parent.uri;
+}

pkgs/native_assets_builder/test/build_runner/concurrency_test.dart

@@ -0,0 +1,223 @@
+// Copyright (c) 2024, 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 'dart:async';
+import 'dart:convert';
+import 'dart:io';
+import 'dart:math';
+
+import 'package:native_assets_builder/src/utils/run_process.dart'
+    show RunProcessResult;
+import 'package:test/test.dart';
+
+import '../helpers.dart';
+import 'helpers.dart';
+
+const Timeout longTimeout = Timeout(Duration(minutes: 5));
+
+void main() async {
+  test('Concurrent invocations', timeout: longTimeout, () async {
+    await inTempDir((tempUri) async {
+      await copyTestProjects(targetUri: tempUri);
+      final packageUri = tempUri.resolve('native_add/');
+
+      await runPubGet(
+        workingDirectory: packageUri,
+        logger: logger,
+      );
+
+      Future<RunProcessResult> runBuildInProcess() async {
+        final result = await runProcess(
+          executable: dartExecutable,
+          arguments: [
+            pkgNativeAssetsBuilderUri
+                .resolve('test/build_runner/concurrency_test_helper.dart')
+                .toFilePath(),
+            packageUri.toFilePath(),
+          ],
+          workingDirectory: packageUri,
+          logger: logger,
+        );
+        expect(result.exitCode, 0);
+        return result;
+      }
+
+      // Simulate running `dart run` concurrently in 3 different terminals.
+      await Future.wait([
+        runBuildInProcess(),
+        runBuildInProcess(),
+        runBuildInProcess(),
+      ]);
+    });
+  });
+
+  File? findLockFile(Uri packageUri) {
+    final dir = Directory.fromUri(
+        packageUri.resolve('.dart_tool/native_assets_builder/'));
+    if (!dir.existsSync()) {
+      // Too quick, dir doesn't exist yet.
+      return null;
+    }
+    for (final entity in dir.listSync().whereType<Directory>()) {
+      final lockFile = File.fromUri(entity.uri.resolve('.lock'));
+      if (lockFile.existsSync()) {
+        if (!Platform.isWindows) {
+          final lockFileContents = lockFile.readAsStringSync();
+          expect(lockFileContents, stringContainsInOrder(['Last acquired by']));
+        }
+        return lockFile;
+      }
+    }
+    return null;
+  }
+
+  test('Terminations unlock', timeout: longTimeout, () async {
+    await inTempDir((tempUri) async {
+      await copyTestProjects(targetUri: tempUri);
+      final packageUri = tempUri.resolve('native_add/');
+
+      await runPubGet(
+        workingDirectory: packageUri,
+        logger: logger,
+      );
+
+      Future<int> runBuildInProcess({Duration? killAfter}) async {
+        final process = await Process.start(
+          dartExecutable.toFilePath(),
+          [
+            pkgNativeAssetsBuilderUri
+                .resolve('test/build_runner/concurrency_test_helper.dart')
+                .toFilePath(),
+            packageUri.toFilePath(),
+          ],
+          workingDirectory: packageUri.toFilePath(),
+        );
+        final stdoutSub = process.stdout
+            .transform(utf8.decoder)
+            .transform(const LineSplitter())
+            .listen(logger.fine);
+        final stderrSub = process.stderr
+            .transform(utf8.decoder)
+            .transform(const LineSplitter())
+            .listen(logger.severe);
+
+        Timer? timer;
+        if (killAfter != null) {
+          timer = Timer(killAfter, process.kill);
+        }
+        final (exitCode, _, _) = await (
+          process.exitCode,
+          stdoutSub.asFuture<void>(),
+          stderrSub.asFuture<void>()
+        ).wait;
+        if (timer != null) {
+          timer.cancel();
+        }
+
+        return exitCode;
+      }
+
+      // Simulate hitting ctrl+c on `dart` and `flutter` commands at different
+      // time intervals.
+      var milliseconds = 200;
+      while (findLockFile(packageUri) == null) {
+        final result = await runBuildInProcess(
+          killAfter: Duration(milliseconds: milliseconds),
+        );
+        expect(result, isNot(0));
+        milliseconds = max((milliseconds * 1.2).round(), milliseconds + 200);
+      }
+      expect(findLockFile(packageUri), isNotNull);
+
+      final result2 = await runBuildInProcess();
+      expect(result2, 0);
+    });
+  });
+
+  test('Timeout exits process', timeout: longTimeout, () async {
+    await inTempDir((tempUri) async {
+      await copyTestProjects(targetUri: tempUri);
+      final packageUri = tempUri.resolve('native_add/');
+
+      await runPubGet(
+        workingDirectory: packageUri,
+        logger: logger,
+      );
+
+      Future<RunProcessResult> runBuildInProcess({
+        Duration? timeout,
+        bool expectTimeOut = false,
+      }) async {
+        final result = await runProcess(
+          executable: dartExecutable,
+          arguments: [
+            pkgNativeAssetsBuilderUri
+                .resolve('test/build_runner/concurrency_test_helper.dart')
+                .toFilePath(),
+            packageUri.toFilePath(),
+            if (timeout != null) timeout.inSeconds.toString(),
+          ],
+          workingDirectory: packageUri,
+          logger: logger,
+        );
+        if (expectTimeOut) {
+          expect(result.exitCode, isNot(0));
+        } else {
+          expect(result.exitCode, 0);
+        }
+
+        return result;
+      }
+
+      await runBuildInProcess();
+
+      final lockFile = findLockFile(packageUri);
+      expect(lockFile, isNotNull);
+      lockFile!;
+
+      // Check how long a cached build takes.
+      final s = Stopwatch();
+      s.start();
+      await runBuildInProcess();
+      s.stop();
+      final cachedInvocationDuration = s.elapsed;
+      final singleHookTimeout = Duration(
+        milliseconds: min(
+          cachedInvocationDuration.inMilliseconds * 2,
+          cachedInvocationDuration.inMilliseconds + 2000,
+        ),
+      );
+      final helperTimeout = Duration(
+        milliseconds: min(
+          singleHookTimeout.inMilliseconds * 2,
+          singleHookTimeout.inMilliseconds + 2000,
+        ),
+      );
+
+      final randomAccessFile = await lockFile.open(mode: FileMode.write);
+      final lock = await randomAccessFile.lock(FileLock.exclusive);
+      var helperCompletedFirst = false;
+      var timeoutCompletedFirst = false;
+      final timer = Timer(helperTimeout, () async {
+        printOnFailure('timer expired');
+        if (!helperCompletedFirst) {
+          timeoutCompletedFirst = true;
+        }
+        await lock.unlock();
+      });
+      await runBuildInProcess(
+        timeout: singleHookTimeout,
+        expectTimeOut: true,
+      ).then((v) async {
+        printOnFailure('helper exited');
+        if (!timeoutCompletedFirst) {
+          helperCompletedFirst = true;
+        }
+        timer.cancel();
+      });
+      expect(helperCompletedFirst, isTrue);
+      expect(timeoutCompletedFirst, isFalse);
+    });
+  });
+}