Services: Track when services are not needed

src/execution/base.ts

@@ -6,6 +6,7 @@
 
 import {shuffle} from '../util/shuffle.js';
 import {Fingerprint} from '../fingerprint.js';
+import {Deferred} from '../util/deferred.js';
 
 import type {Result} from '../error.js';
 import type {Executor} from '../executor.js';
@@ -84,3 +85,21 @@ export abstract class BaseExecution<T extends ScriptConfig> {
     return {ok: true, value: results};
   }
 }
+
+/**
+ * A single execution of a specific script which has a command.
+ */
+export abstract class BaseExecutionWithCommand<
+  T extends ScriptConfig & {
+    command: Exclude<ScriptConfig['command'], undefined>;
+  }
+> extends BaseExecution<T> {
+  protected readonly _servicesNotNeeded = new Deferred<void>();
+
+  /**
+   * Resolves when this script no longer needs any of its service dependencies
+   * to be running. This could happen because it finished, failed, or never
+   * needed to run at all.
+   */
+  readonly servicesNotNeeded = this._servicesNotNeeded.promise;
+}

src/execution/service.ts

@@ -4,16 +4,28 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import {BaseExecution} from './base.js';
+import {BaseExecutionWithCommand} from './base.js';
 import {Fingerprint} from '../fingerprint.js';
 
 import type {ExecutionResult} from './base.js';
 import type {ServiceScriptConfig} from '../config.js';
+import type {Executor} from '../executor.js';
+import type {Logger} from '../logging/logger.js';
 
 /**
  * Execution for a {@link ServiceScriptConfig}.
  */
-export class ServiceScriptExecution extends BaseExecution<ServiceScriptConfig> {
+export class ServiceScriptExecution extends BaseExecutionWithCommand<ServiceScriptConfig> {
+  constructor(
+    config: ServiceScriptConfig,
+    executor: Executor,
+    logger: Logger,
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    _abort: Promise<void>
+  ) {
+    super(config, executor, logger);
+  }
+
   /**
    * Note `execute` is a bit of a misnomer here, because we don't actually
    * execute the command at this stage in the case of services.

src/execution/standard.ts

@@ -13,7 +13,7 @@ import {glob, GlobOutsideCwdError} from '../util/glob.js';
 import {deleteEntries} from '../util/delete.js';
 import lockfile from 'proper-lockfile';
 import {ScriptChildProcess} from '../script-child-process.js';
-import {BaseExecution} from './base.js';
+import {BaseExecutionWithCommand} from './base.js';
 import {Fingerprint} from '../fingerprint.js';
 import {computeManifestEntry} from '../util/manifest.js';
 
@@ -36,7 +36,7 @@ type StandardScriptExecutionState =
 /**
  * Execution for a {@link StandardScriptConfig}.
  */
-export class StandardScriptExecution extends BaseExecution<StandardScriptConfig> {
+export class StandardScriptExecution extends BaseExecutionWithCommand<StandardScriptConfig> {
   private _state: StandardScriptExecutionState = 'before-running';
   private readonly _cache?: Cache;
   private readonly _workerPool: WorkerPool;
@@ -60,57 +60,61 @@ export class StandardScriptExecution extends BaseExecution<StandardScriptConfig>
   }
 
   protected async _execute(): Promise<ExecutionResult> {
-    this._ensureState('before-running');
-
-    const dependencyFingerprints = await this._executeDependencies();
-    if (!dependencyFingerprints.ok) {
-      dependencyFingerprints.error.push(this._startCancelledEvent);
-      return dependencyFingerprints;
-    }
-
-    // Significant time could have elapsed since we last checked because our
-    // dependencies had to finish.
-    if (this._shouldNotStart) {
-      return {ok: false, error: [this._startCancelledEvent]};
-    }
+    try {
+      this._ensureState('before-running');
 
-    return this._acquireSystemLockIfNeeded(async () => {
-      // Note we must wait for dependencies to finish before generating the
-      // cache key, because a dependency could create or modify an input file to
-      // this script, which would affect the key.
-      const fingerprint = await Fingerprint.compute(
-        this._config,
-        dependencyFingerprints.value
-      );
-      if (await this._fingerprintIsFresh(fingerprint)) {
-        const manifestFresh = await this._outputManifestIsFresh();
-        if (!manifestFresh.ok) {
-          return {ok: false, error: [manifestFresh.error]};
-        }
-        if (manifestFresh.value) {
-          return this._handleFresh(fingerprint);
-        }
+      const dependencyFingerprints = await this._executeDependencies();
+      if (!dependencyFingerprints.ok) {
+        dependencyFingerprints.error.push(this._startCancelledEvent);
+        return dependencyFingerprints;
       }
 
-      // Computing the fingerprint can take some time, and the next operation is
-      // destructive. Another good opportunity to check if we should still
-      // start.
+      // Significant time could have elapsed since we last checked because our
+      // dependencies had to finish.
       if (this._shouldNotStart) {
         return {ok: false, error: [this._startCancelledEvent]};
       }
 
-      const cacheHit = fingerprint.data.fullyTracked
-        ? await this._cache?.get(this._config, fingerprint)
-        : undefined;
-      if (this._shouldNotStart) {
-        return {ok: false, error: [this._startCancelledEvent]};
-      }
-      if (cacheHit !== undefined) {
-        return this._handleCacheHit(cacheHit, fingerprint);
-      }
+      return this._acquireSystemLockIfNeeded(async () => {
+        // Note we must wait for dependencies to finish before generating the
+        // cache key, because a dependency could create or modify an input file to
+        // this script, which would affect the key.
+        const fingerprint = await Fingerprint.compute(
+          this._config,
+          dependencyFingerprints.value
+        );
+        if (await this._fingerprintIsFresh(fingerprint)) {
+          const manifestFresh = await this._outputManifestIsFresh();
+          if (!manifestFresh.ok) {
+            return {ok: false, error: [manifestFresh.error]};
+          }
+          if (manifestFresh.value) {
+            return this._handleFresh(fingerprint);
+          }
+        }
 
-      return this._handleNeedsRun(fingerprint);
-    });
+        // Computing the fingerprint can take some time, and the next operation is
+        // destructive. Another good opportunity to check if we should still
+        // start.
+        if (this._shouldNotStart) {
+          return {ok: false, error: [this._startCancelledEvent]};
+        }
+
+        const cacheHit = fingerprint.data.fullyTracked
+          ? await this._cache?.get(this._config, fingerprint)
+          : undefined;
+        if (this._shouldNotStart) {
+          return {ok: false, error: [this._startCancelledEvent]};
+        }
+        if (cacheHit !== undefined) {
+          return this._handleCacheHit(cacheHit, fingerprint);
+        }
+
+        return this._handleNeedsRun(fingerprint);
+      });
+    } finally {
+      this._servicesNotNeeded.resolve();
+    }
   }
 
   /**
@@ -239,6 +243,10 @@ export class StandardScriptExecution extends BaseExecution<StandardScriptConfig>
     cacheHit: CacheHit,
     fingerprint: Fingerprint
   ): Promise<ExecutionResult> {
+    // Optimization: early signal that services are not needed while we're still
+    // restoring from cache.
+    this._servicesNotNeeded.resolve();
+
     // Delete the fingerprint and other files. It's important we do this before
     // restoring from cache, because we don't want to think that the previous
     // fingerprint is still valid when it no longer is.
@@ -373,6 +381,10 @@ export class StandardScriptExecution extends BaseExecution<StandardScriptConfig>
       return {ok: false, error: [childResult.error]};
     }
 
+    // Optimization: early signal that services are no longer needed while we're
+    // still writing the fingerprint file etc.
+    this._servicesNotNeeded.resolve();
+
     const writeFingerprintPromise = this._writeFingerprintFile(fingerprint);
     const outputFilesAfterRunning = await this._globOutputFilesAfterRunning();
     if (!outputFilesAfterRunning.ok) {

src/executor.ts

@@ -51,6 +51,7 @@ export class Executor {
   private readonly _logger: Logger;
   private readonly _workerPool: WorkerPool;
   private readonly _cache?: Cache;
+  private readonly _abort: Deferred<void>;
 
   /** Resolves when the first failure occurs in any script. */
   private readonly _failureOccured = new Deferred<void>();
@@ -69,6 +70,7 @@ export class Executor {
     this._logger = logger;
     this._workerPool = workerPool;
     this._cache = cache;
+    this._abort = abort;
 
     // If this entire execution is aborted because e.g. the user sent a SIGINT
     // to the Wireit process, then dont start new scripts, and kill running
@@ -140,7 +142,12 @@ export class Executor {
       if (config.command === undefined) {
         execution = new NoCommandScriptExecution(config, this, this._logger);
       } else if (config.service) {
-        execution = new ServiceScriptExecution(config, this, this._logger);
+        execution = new ServiceScriptExecution(
+          config,
+          this,
+          this._logger,
+          this._abort.promise
+        );
       } else {
         execution = new StandardScriptExecution(
           config,

src/watcher.ts

@@ -48,11 +48,11 @@ type WatcherState =
   | 'aborted';
 
 function unknownState(state: never) {
-  throw new Error(`Unknown watcher state ${String(state)}`);
+  return new Error(`Unknown watcher state ${String(state)}`);
 }
 
 function unexpectedState(state: WatcherState) {
-  throw new Error(`Unexpected watcher state ${state}`);
+  return new Error(`Unexpected watcher state ${state}`);
 }
 
 /**