Retry build_daemon state file reads

analysis_options.yaml

@@ -49,7 +49,7 @@ linter:
     #- implementation_imports
     - invariant_booleans
     - iterable_contains_unrelated_type
-    - join_return_with_assignment
+    #- join_return_with_assignment
     - library_names
     - library_prefixes
     - list_remove_unrelated_type

build_daemon/CHANGELOG.md

@@ -1,5 +1,6 @@
 ## 0.6.0
 
+- Add retry logic to the state file helpers `runningVersion` and `currentOptions`.
 - `DaemonBuilder` is now an abstract class.
 - Significantly increase doc comment coverage.
 

build_daemon/lib/client.dart

@@ -16,10 +16,11 @@ import 'data/build_status.dart';
 import 'data/build_target_request.dart';
 import 'data/serializers.dart';
 import 'data/server_log.dart';
+import 'src/file_wait.dart';
 
-int _existingPort(String workingDirectory) {
+Future<int> _existingPort(String workingDirectory) async {
   var portFile = File(portFilePath(workingDirectory));
-  if (!portFile.existsSync()) throw MissingPortFile();
+  if (!await waitForFile(portFile)) throw MissingPortFile();
   return int.parse(portFile.readAsStringSync());
 }
 
@@ -141,7 +142,7 @@ class BuildDaemonClient {
     await _handleDaemonStartup(process, logHandler);
 
     return BuildDaemonClient._(
-        _existingPort(workingDirectory), daemonSerializers, logHandler);
+        await _existingPort(workingDirectory), daemonSerializers, logHandler);
   }
 }
 

build_daemon/lib/constants.dart

@@ -11,7 +11,7 @@ const versionSkew = 'DIFFERENT RUNNING VERSION';
 const optionsSkew = 'DIFFERENT OPTIONS';
 
 // TODO(grouma) - use pubspec version when this is open sourced.
-const currentVersion = '5.0.0';
+const currentVersion = '6.0.0';
 
 var _username = Platform.environment['USER'] ?? '';
 String daemonWorkspace(String workingDirectory) {

build_daemon/lib/src/daemon.dart

@@ -12,23 +12,24 @@ import 'package:watcher/watcher.dart';
 import '../constants.dart';
 import '../daemon_builder.dart';
 import '../data/build_target.dart';
+import 'file_wait.dart';
 import 'server.dart';
 
 /// Returns the current version of the running build daemon.
 ///
 /// Null if one isn't running.
-String runningVersion(String workingDirectory) {
+Future<String> runningVersion(String workingDirectory) async {
   var versionFile = File(versionFilePath(workingDirectory));
-  if (!versionFile.existsSync()) return null;
+  if (!await waitForFile(versionFile)) return null;
   return versionFile.readAsStringSync();
 }
 
 /// Returns the current options of the running build daemon.
 ///
 /// Null if one isn't running.
-Set<String> currentOptions(String workingDirectory) {
+Future<Set<String>> currentOptions(String workingDirectory) async {
   var optionsFile = File(optionsFilePath(workingDirectory));
-  if (!optionsFile.existsSync()) return Set();
+  if (!await waitForFile(optionsFile)) return Set();
   return optionsFile.readAsLinesSync().toSet();
 }
 

build_daemon/lib/src/file_wait.dart

@@ -0,0 +1,24 @@
+// 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 'dart:async';
+import 'dart:io';
+
+const _readDelay = Duration(milliseconds: 100);
+const _maxWait = Duration(seconds: 5);
+
+/// Returns true if the file exists.
+///
+/// If the file does not exist it keeps retrying for a period of time.
+/// Returns false if the file never becomes available.
+///
+/// This reduces the likelihood of race conditions.
+Future<bool> waitForFile(File file) async {
+  final end = DateTime.now().add(_maxWait);
+  while (!DateTime.now().isAfter(end)) {
+    if (file.existsSync()) return true;
+    await Future.delayed(_readDelay);
+  }
+  return file.existsSync();
+}

build_daemon/test/daemon_test.dart

@@ -50,7 +50,7 @@ void main() {
       var workspace = uuid.v1();
       var daemon = await _runDaemon(workspace);
       testDaemons.add(daemon);
-      expect(await getOutput(daemon), 'RUNNING');
+      expect(await _statusOf(daemon), 'RUNNING');
     });
 
     test('shuts down if no client connects', () async {
@@ -65,10 +65,10 @@ void main() {
       var workspace = uuid.v1();
       testWorkspaces.add(workspace);
       var daemonOne = await _runDaemon(workspace);
-      expect(await getOutput(daemonOne), 'RUNNING');
+      expect(await _statusOf(daemonOne), 'RUNNING');
       var daemonTwo = await _runDaemon(workspace);
       testDaemons.addAll([daemonOne, daemonTwo]);
-      expect(await getOutput(daemonTwo), 'ALREADY RUNNING');
+      expect(await _statusOf(daemonTwo), 'ALREADY RUNNING');
     });
 
     test('can run if another daemon is running in a different workspace',
@@ -77,48 +77,58 @@ void main() {
       var workspace2 = uuid.v1();
       testWorkspaces.addAll([workspace1, workspace2]);
       var daemonOne = await _runDaemon(workspace1);
-      expect(await getOutput(daemonOne), 'RUNNING');
+      expect(await _statusOf(daemonOne), 'RUNNING');
       var daemonTwo = await _runDaemon(workspace2);
       testDaemons.addAll([daemonOne, daemonTwo]);
-      expect(await getOutput(daemonTwo), 'RUNNING');
+      expect(await _statusOf(daemonTwo), 'RUNNING');
+    });
+
+    test('can start two daemons at the same time', () async {
+      var workspace = uuid.v1();
+      testWorkspaces.add(workspace);
+      var daemonOne = await _runDaemon(workspace);
+      var daemonTwo = await _runDaemon(workspace);
+      expect([await _statusOf(daemonOne), await _statusOf(daemonTwo)],
+          containsAll(['RUNNING', 'ALREADY RUNNING']));
+      testDaemons.addAll([daemonOne, daemonTwo]);
     });
 
     test('logs the version when running', () async {
       var workspace = uuid.v1();
       testWorkspaces.add(workspace);
       var daemon = await _runDaemon(workspace);
       testDaemons.add(daemon);
-      expect(await getOutput(daemon), 'RUNNING');
-      expect(runningVersion(workspace), currentVersion);
+      expect(await _statusOf(daemon), 'RUNNING');
+      expect(await runningVersion(workspace), currentVersion);
     });
 
     test('does not set the current version if not running', () async {
       var workspace = uuid.v1();
       testWorkspaces.add(workspace);
-      expect(runningVersion(workspace), null);
+      expect(await runningVersion(workspace), null);
     });
 
     test('logs the options when running', () async {
       var workspace = uuid.v1();
       testWorkspaces.add(workspace);
       var daemon = await _runDaemon(workspace);
       testDaemons.add(daemon);
-      expect(await getOutput(daemon), 'RUNNING');
-      expect(currentOptions(workspace).contains('foo'), isTrue);
+      expect(await _statusOf(daemon), 'RUNNING');
+      expect((await currentOptions(workspace)).contains('foo'), isTrue);
     });
 
     test('does not log the options if not running', () async {
       var workspace = uuid.v1();
       testWorkspaces.add(workspace);
-      expect(currentOptions(workspace).isEmpty, isTrue);
+      expect((await currentOptions(workspace)).isEmpty, isTrue);
     });
 
     test('cleans up after itself', () async {
       var workspace = uuid.v1();
       testWorkspaces.add(workspace);
       var daemon = await _runDaemon(workspace);
       // Wait for the daemon to be running before checking the workspace exits.
-      expect(await getOutput(daemon), 'RUNNING');
+      expect(await _statusOf(daemon), 'RUNNING');
       expect(Directory(daemonWorkspace(workspace)).existsSync(), isTrue);
       // Daemon expects sigint twice before quitting.
       daemon..kill(ProcessSignal.sigint)..kill(ProcessSignal.sigint);
@@ -128,28 +138,45 @@ void main() {
   });
 }
 
-Future<String> getOutput(Process daemon) async {
-  return await daemon.stdout
+/// Returns the daemon status.
+///
+/// If the status is null, the stderr ir returned.
+Future<String> _statusOf(Process daemon) async {
+  var status = await daemon.stdout
       .transform(utf8.decoder)
       .transform(const LineSplitter())
-      .firstWhere((line) => line == 'RUNNING' || line == 'ALREADY RUNNING');
+      .firstWhere((line) => line == 'RUNNING' || line == 'ALREADY RUNNING',
+          orElse: () => null);
+  status ??= (await daemon.stderr
+          .transform(utf8.decoder)
+          .transform(const LineSplitter())
+          .toList())
+      .join('\n');
+  return status;
 }
 
 Future<Process> _runDaemon(var workspace, {int timeout = 30}) async {
   await d.file('test.dart', '''
     import 'package:build_daemon/src/daemon.dart';
     import 'package:build_daemon/src/fake_builder.dart';
     import 'package:build_daemon/daemon_builder.dart';
+    import 'package:build_daemon/client.dart';
 
     main() async {
       var daemon = Daemon('$workspace');
       if (daemon.tryGetLock()) {
         var options = ['foo'].toSet();
         var timeout = Duration(seconds: $timeout);
+        // Real implementations of the daemon usually non-trivial set up time
+        // before calling start.
+        await Future.delayed(Duration(seconds: 1));
         await daemon.start(options, FakeDaemonBuilder(), Stream.empty(),
         timeout: timeout);
         print('RUNNING');
       } else {
+        // Mimic the behavior of actual daemon implementations.
+        var version = await runningVersion('$workspace');
+        if(version != '$currentVersion') throw VersionSkew();
         print('ALREADY RUNNING');
       }
     }

build_runner/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.3.5
+
+- Use the latest `build_daemon`.
+
 ## 1.3.4
 
 - Use the latest `build_config`.

build_runner/lib/src/entrypoint/daemon.dart

@@ -32,8 +32,8 @@ class DaemonCommand extends BuildRunnerCommand {
     var daemon = Daemon(workingDirectory);
     var requestedOptions = argResults.arguments.toSet();
     if (!daemon.tryGetLock()) {
-      var runningOptions = currentOptions(workingDirectory);
-      var version = runningVersion(workingDirectory);
+      var runningOptions = await currentOptions(workingDirectory);
+      var version = await runningVersion(workingDirectory);
       if (version != currentVersion) {
         stdout
           ..writeln('Running Version: $version')

build_runner/pubspec.yaml

@@ -1,5 +1,5 @@
 name: build_runner
-version: 1.3.4
+version: 1.3.5
 description: Tools to write binaries that run builders.
 author: Dart Team <misc@dartlang.org>
 homepage: https://github.com/dart-lang/build/tree/master/build_runner
@@ -12,7 +12,7 @@ dependencies:
   async: ">=1.13.3 <3.0.0"
   build: ">=1.0.0 <1.2.0"
   build_config: ">=0.4.0 <0.4.1"
-  build_daemon: ^0.5.0
+  build_daemon: ^0.6.0
   build_resolvers: "^1.0.0"
   build_runner_core: ^3.0.0
   code_builder: ">2.3.0 <4.0.0"
@@ -59,3 +59,5 @@ dependency_overrides:
     path: ../build_config
   build_runner_core:
     path: ../build_runner_core
+  build_daemon:
+    path: ../build_daemon