diff --git a/src/execution/base.ts b/src/execution/base.ts index 946411aa3..13647b0e5 100644 --- a/src/execution/base.ts +++ b/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 { 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; + } +> extends BaseExecution { + protected readonly _servicesNotNeeded = new Deferred(); + + /** + * 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; +} diff --git a/src/execution/service.ts b/src/execution/service.ts index b2e91dfec..499d6885c 100644 --- a/src/execution/service.ts +++ b/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 { +export class ServiceScriptExecution extends BaseExecutionWithCommand { + constructor( + config: ServiceScriptConfig, + executor: Executor, + logger: Logger, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _abort: Promise + ) { + 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. diff --git a/src/execution/standard.ts b/src/execution/standard.ts index adaef1acc..920303d44 100644 --- a/src/execution/standard.ts +++ b/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 { +export class StandardScriptExecution extends BaseExecutionWithCommand { private _state: StandardScriptExecutionState = 'before-running'; private readonly _cache?: Cache; private readonly _workerPool: WorkerPool; @@ -60,57 +60,61 @@ export class StandardScriptExecution extends BaseExecution } protected async _execute(): Promise { - 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 cacheHit: CacheHit, fingerprint: Fingerprint ): Promise { + // 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 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) { diff --git a/src/executor.ts b/src/executor.ts index 0edcc73d0..b800a2dcf 100644 --- a/src/executor.ts +++ b/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; /** Resolves when the first failure occurs in any script. */ private readonly _failureOccured = new Deferred(); @@ -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, diff --git a/src/watcher.ts b/src/watcher.ts index f5290336e..af18a461b 100644 --- a/src/watcher.ts +++ b/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}`); } /**