From 55e1bf44945cb6d0815f97b67884511a24abe5db Mon Sep 17 00:00:00 2001 From: Bharat Middha <5100938+bmiddha@users.noreply.github.com> Date: Mon, 20 Jul 2026 23:19:50 +0000 Subject: [PATCH 1/3] [WS0-T4] Per-iteration host-driven runner persist policy Move IPC runner teardown into the operation graph's per-iteration options while preserving resident runners when no policy is supplied. Cover persistent, one-shot, changing, and default policies across successive iterations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8f21c8c8-452a-4922-aca3-fb40b007f948 --- .../rush/bmiddha-runner-persist-policy.json | 9 +++ common/reviews/api/rush-lib.api.md | 1 + .../src/logic/operations/IOperationGraph.ts | 9 +++ .../logic/operations/IPCOperationRunner.ts | 10 --- .../operations/IPCOperationRunnerPlugin.ts | 1 - .../src/logic/operations/OperationGraph.ts | 29 ++++++- .../operations/test/OperationGraph.test.ts | 79 +++++++++++++++++-- 7 files changed, 118 insertions(+), 20 deletions(-) create mode 100644 common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json diff --git a/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json b/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json new file mode 100644 index 00000000000..c084a6f1098 --- /dev/null +++ b/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json @@ -0,0 +1,9 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Add a per-iteration, host-driven runner persist policy so IPC runners can be kept hot or torn down per operation per iteration, instead of fixing persistence at plugin construction.", + "type": "minor" + } + ] +} diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 6f21ecfaec3..4d1eebbb418 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -651,6 +651,7 @@ export interface IOperationGraphContext extends ICreateOperationsContext { // @alpha export interface IOperationGraphIterationOptions { + getRunnerPersistence?: (operation: Operation) => boolean; // (undocumented) inputsSnapshot?: IInputsSnapshot; startTime?: number; diff --git a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts index 294a6205675..69c3f43f15d 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts @@ -21,6 +21,15 @@ export interface IOperationGraphIterationOptions { * The time when the iteration was scheduled, if available, as returned by `performance.now()`. */ startTime?: number; + + /** + * Returns whether the operation's runner should remain active after this iteration. + * + * @remarks + * When omitted, all runners remain active. Returning `false` causes the operation's runner to be closed + * after the iteration, including when the operation was disabled for that iteration. + */ + getRunnerPersistence?: (operation: Operation) => boolean; } /** diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts index 6d8396591c5..2ad46169a42 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts @@ -28,7 +28,6 @@ export interface IIPCOperationRunnerOptions { initialCommand: string; incrementalCommand: string | undefined; commandForHash: string; - persist: boolean; ignoredParameterValues: ReadonlyArray; } @@ -58,7 +57,6 @@ export class IPCOperationRunner implements IOperationRunner { private readonly _initialCommand: string; private readonly _incrementalCommand: string | undefined; private readonly _commandForHash: string; - private readonly _persist: boolean; private readonly _ignoredParameterValues: ReadonlyArray; private _ipcProcess: ChildProcess | undefined; @@ -72,7 +70,6 @@ export class IPCOperationRunner implements IOperationRunner { initialCommand, incrementalCommand, commandForHash, - persist, ignoredParameterValues } = options; this.name = name; @@ -83,7 +80,6 @@ export class IPCOperationRunner implements IOperationRunner { this._incrementalCommand = incrementalCommand; this._commandForHash = commandForHash; - this._persist = persist; this._ignoredParameterValues = ignoredParameterValues; } @@ -100,7 +96,6 @@ export class IPCOperationRunner implements IOperationRunner { const invalidate: (reason: string) => void = context.getInvalidateCallback(); return await context.runWithTerminalAsync( async (terminal: ITerminal, terminalProvider: ITerminalProvider): Promise => { - let isConnected: boolean = false; if (!this._ipcProcess || typeof this._ipcProcess.exitCode === 'number') { // Log any ignored parameters if (this._ignoredParameterValues.length > 0) { @@ -204,7 +199,6 @@ export class IPCOperationRunner implements IOperationRunner { subProcess.on('exit', onExit); this._processReadyPromise!.then(() => { - isConnected = true; terminal.writeLine('Child supports IPC protocol. Sending "run" command...'); const runCommand: IRunCommandMessage = { command: 'run' @@ -213,10 +207,6 @@ export class IPCOperationRunner implements IOperationRunner { }, reject); }); - if (isConnected && !this._persist) { - await this.closeAsync(); - } - // @rushstack/operation-graph does not currently have a concept of "Success with Warning" // To match existing ShellOperationRunner behavior we treat any stderr as a warning. return status === OperationStatus.Success && hasWarningOrError diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts index 7d5cccd7851..1823c22b4f1 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts @@ -81,7 +81,6 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin { initialCommand, incrementalCommand, commandForHash, - persist: true, ignoredParameterValues }); diff --git a/libraries/rush-lib/src/logic/operations/OperationGraph.ts b/libraries/rush-lib/src/logic/operations/OperationGraph.ts index 8d01d746186..9ab09fe56a6 100644 --- a/libraries/rush-lib/src/logic/operations/OperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/OperationGraph.ts @@ -85,6 +85,7 @@ interface IExecutionIterationContext extends IOperationExecutionRecordContext { promise: Promise | undefined; startTime?: number; + getRunnerPersistence: IOperationGraphIterationOptions['getRunnerPersistence']; completedOperations: number; totalOperations: number; @@ -572,9 +573,16 @@ export class OperationGraph implements IOperationGraph { ): Promise { const { _getInputsSnapshotAsync: getInputsSnapshotAsync } = this; - const { startTime = performance.now(), inputsSnapshot = await getInputsSnapshotAsync?.() } = - iterationOptions; - const iterationOptionsForCallbacks: IOperationGraphIterationOptions = { startTime, inputsSnapshot }; + const { + startTime = performance.now(), + inputsSnapshot = await getInputsSnapshotAsync?.(), + getRunnerPersistence + } = iterationOptions; + const iterationOptionsForCallbacks: IOperationGraphIterationOptions = { + startTime, + inputsSnapshot, + getRunnerPersistence + }; const { hooks } = this; @@ -607,6 +615,7 @@ export class OperationGraph implements IOperationGraph { const iterationContext: IExecutionIterationContext = { abortController, startTime, + getRunnerPersistence, streamCollator, terminal, inputsSnapshot, @@ -757,7 +766,8 @@ export class OperationGraph implements IOperationGraph { const iterationOptions: IOperationGraphIterationOptions = { inputsSnapshot: iterationContext.inputsSnapshot, - startTime: iterationContext.startTime + startTime: iterationContext.startTime, + getRunnerPersistence: iterationContext.getRunnerPersistence }; const executionQueue: AsyncOperationQueue = new AsyncOperationQueue( @@ -1011,6 +1021,17 @@ export class OperationGraph implements IOperationGraph { telemetry.log(logEntry); } + const { getRunnerPersistence } = iterationContext; + if (getRunnerPersistence) { + const operationsToClose: Operation[] = []; + for (const operation of this.operations) { + if (!getRunnerPersistence(operation)) { + operationsToClose.push(operation); + } + } + await this.closeRunnersAsync(operationsToClose); + } + return status; // This function is a callback because it may write to the collatedWriter before diff --git a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts index 913f15f705a..a4381421efc 100644 --- a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts +++ b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts @@ -107,6 +107,12 @@ function createGraph( return new OperationGraph(new Set([operation]), graphOptions); } +class ClosableRunner extends MockOperationRunner { + public readonly closeAsync: jest.Mock, []> = jest.fn(async () => { + /* no-op */ + }); +} + describe('OperationGraph', () => { let graphOptions: IOperationGraphOptions; let graphIterationOptions: IOperationGraphIterationOptions; @@ -1134,13 +1140,76 @@ describe('deferred invalidation during active iteration', () => { }); }); -describe('closeRunnersAsync', () => { - class ClosableRunner extends MockOperationRunner { - public readonly closeAsync: jest.Mock, []> = jest.fn(async () => { - /* no-op */ +describe('runner persistence policy', () => { + const graphOptions: IOperationGraphOptions = { + quietMode: false, + debugMode: false, + parallelism: 3, + allowOversubscription: true, + destinations: [mockWritable], + abortController: new AbortController() + }; + + it('keeps runners active across successive iterations when no policy is provided', async () => { + const runAsync: jest.Mock, []> = jest.fn(async () => OperationStatus.Success); + const runner: ClosableRunner = new ClosableRunner('default-persistent', runAsync); + const graph: OperationGraph = createGraph(graphOptions, runner); + + await graph.executeAsync({}); + expect(runAsync).toHaveBeenCalledTimes(1); + expect(runner.closeAsync).not.toHaveBeenCalled(); + + await graph.executeAsync({}); + expect(runAsync).toHaveBeenCalledTimes(2); + expect(runner.closeAsync).not.toHaveBeenCalled(); + }); + + it('applies per-operation policies independently on each iteration', async () => { + const persistentRunner: ClosableRunner = new ClosableRunner('persistent'); + const oneShotRunner: ClosableRunner = new ClosableRunner('one-shot'); + const changingRunner: ClosableRunner = new ClosableRunner('changing'); + + const persistentOperation: Operation = new Operation({ + runner: persistentRunner, + logFilenameIdentifier: 'persistent', + phase: mockPhase, + project: getOrCreateProject('persistent') + }); + const oneShotOperation: Operation = new Operation({ + runner: oneShotRunner, + logFilenameIdentifier: 'one-shot', + phase: mockPhase, + project: getOrCreateProject('one-shot') + }); + const changingOperation: Operation = new Operation({ + runner: changingRunner, + logFilenameIdentifier: 'changing', + phase: mockPhase, + project: getOrCreateProject('changing') }); - } + const graph: OperationGraph = new OperationGraph( + new Set([persistentOperation, oneShotOperation, changingOperation]), + graphOptions + ); + + await graph.executeAsync({ + getRunnerPersistence: (operation) => operation !== oneShotOperation + }); + expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); + expect(oneShotRunner.closeAsync).toHaveBeenCalledTimes(1); + expect(changingRunner.closeAsync).not.toHaveBeenCalled(); + + await graph.executeAsync({ + getRunnerPersistence: (operation) => operation === persistentOperation + }); + expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); + expect(oneShotRunner.closeAsync).toHaveBeenCalledTimes(2); + expect(changingRunner.closeAsync).toHaveBeenCalledTimes(1); + }); +}); + +describe('closeRunnersAsync', () => { it('invokes closeAsync on runners and triggers onExecutionStatesUpdated hook', async () => { const localOptions: IOperationGraphOptions = { quietMode: false, From 94af331f342c79316a925f295461bc759178ff8f Mon Sep 17 00:00:00 2001 From: Bharat Middha <5100938+bmiddha@users.noreply.github.com> Date: Mon, 20 Jul 2026 16:26:16 -0700 Subject: [PATCH 2/3] Apply suggestion from @bmiddha --- .../changes/@microsoft/rush/bmiddha-runner-persist-policy.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json b/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json index c084a6f1098..336d44ae1ae 100644 --- a/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json +++ b/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json @@ -3,7 +3,7 @@ { "packageName": "@microsoft/rush", "comment": "Add a per-iteration, host-driven runner persist policy so IPC runners can be kept hot or torn down per operation per iteration, instead of fixing persistence at plugin construction.", - "type": "minor" + "type": "patch" } ] } From 4854b5bde1d451e8fac9b428afe0c54cdaad6a0a Mon Sep 17 00:00:00 2001 From: Bharat Middha <5100938+bmiddha@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:42:59 +0000 Subject: [PATCH 3/3] Address runner persistence review feedback Close nonpersistent IPC runners before downstream execution, retain fallback cleanup for bypassed active runners, and rename the policy to shouldRunnerPersist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8f21c8c8-452a-4922-aca3-fb40b007f948 --- common/reviews/api/rush-lib.api.md | 3 +- .../src/logic/operations/IOperationGraph.ts | 7 +- .../src/logic/operations/IOperationRunner.ts | 11 ++ .../logic/operations/IPCOperationRunner.ts | 10 ++ .../operations/OperationExecutionRecord.ts | 9 ++ .../src/logic/operations/OperationGraph.ts | 19 +-- .../operations/test/OperationGraph.test.ts | 108 +++++++++++++++++- 7 files changed, 152 insertions(+), 15 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 4d1eebbb418..39f0fced3f8 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -651,9 +651,9 @@ export interface IOperationGraphContext extends ICreateOperationsContext { // @alpha export interface IOperationGraphIterationOptions { - getRunnerPersistence?: (operation: Operation) => boolean; // (undocumented) inputsSnapshot?: IInputsSnapshot; + shouldRunnerPersist?: (operation: Operation) => boolean; startTime?: number; } @@ -722,6 +722,7 @@ export interface IOperationRunnerContext { createLogFile: boolean; logFileSuffix?: string; }): Promise; + shouldRunnerPersist?: boolean; status: OperationStatus; stopwatch: IStopwatchResult; } diff --git a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts index 69c3f43f15d..768fb2c2bcf 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts @@ -26,10 +26,11 @@ export interface IOperationGraphIterationOptions { * Returns whether the operation's runner should remain active after this iteration. * * @remarks - * When omitted, all runners remain active. Returning `false` causes the operation's runner to be closed - * after the iteration, including when the operation was disabled for that iteration. + * When omitted, all runners remain active. Returning `false` causes a runner that supports immediate + * teardown to close when its operation completes. Any cold runner that remains active because its operation + * did not execute is closed after the iteration. */ - getRunnerPersistence?: (operation: Operation) => boolean; + shouldRunnerPersist?: (operation: Operation) => boolean; } /** diff --git a/libraries/rush-lib/src/logic/operations/IOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IOperationRunner.ts index e8dc3f2d10d..5d4b27822df 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationRunner.ts @@ -69,6 +69,15 @@ export interface IOperationRunnerContext { */ error?: Error; + /** + * Whether the host wants this operation's runner to remain resident (kept warm) after the + * operation completes for the current iteration. Defaults to `true` when the host provides no + * persistence policy. Runners that own a long-lived child process (such as `IPCOperationRunner`) + * must release that process as soon as the operation completes when this is `false`, so that the + * subprocess does not consume memory while downstream operations execute. + */ + shouldRunnerPersist?: boolean; + /** * Returns a callback that invalidates this operation so that it will be re-executed in the next iteration. * The returned callback captures only the minimal state needed, avoiding retention of the full context. @@ -134,6 +143,8 @@ export interface IOperationRunner { * If true, this runner currently owns some kind of active resource (e.g. a service or a watch process). * This can be used to determine if the operation is "in progress" even if it is not currently executing. * If the runner supports this property, it should update it as appropriate during execution. + * A runner that closes its resource during `executeAsync()` when `context.shouldRunnerPersist` is false + * must report `false` afterward so that the graph's fallback cleanup does not close it again. * The property is optional to avoid breaking existing implementations of IOperationRunner. */ readonly isActive?: boolean; diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts index 2ad46169a42..21c3acb9e83 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts @@ -96,6 +96,7 @@ export class IPCOperationRunner implements IOperationRunner { const invalidate: (reason: string) => void = context.getInvalidateCallback(); return await context.runWithTerminalAsync( async (terminal: ITerminal, terminalProvider: ITerminalProvider): Promise => { + let isConnected: boolean = false; if (!this._ipcProcess || typeof this._ipcProcess.exitCode === 'number') { // Log any ignored parameters if (this._ignoredParameterValues.length > 0) { @@ -199,6 +200,7 @@ export class IPCOperationRunner implements IOperationRunner { subProcess.on('exit', onExit); this._processReadyPromise!.then(() => { + isConnected = true; terminal.writeLine('Child supports IPC protocol. Sending "run" command...'); const runCommand: IRunCommandMessage = { command: 'run' @@ -207,6 +209,14 @@ export class IPCOperationRunner implements IOperationRunner { }, reject); }); + // If the host does not want this runner kept warm for the next iteration, release the + // long-lived subprocess immediately now that the operation has completed. Deferring this to + // the end of the iteration would keep the subprocess resident (consuming memory) while + // downstream operations execute, risking RAM exhaustion. + if (isConnected && context.shouldRunnerPersist === false) { + await this.closeAsync(); + } + // @rushstack/operation-graph does not currently have a concept of "Success with Warning" // To match existing ShellOperationRunner behavior we treat any stderr as a warning. return status === OperationStatus.Success && hasWarningOrError diff --git a/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts b/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts index 2de7655d1d3..7252795b80d 100644 --- a/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts +++ b/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts @@ -46,6 +46,7 @@ export interface IOperationExecutionRecordContext { onOperationStateChanged?: (record: OperationExecutionRecord) => void; createEnvironment?: (record: OperationExecutionRecord) => IEnvironment; invalidate?: (operations: Iterable, reason: string) => void; + shouldRunnerPersist?: (operation: Operation) => boolean; inputsSnapshot: IInputsSnapshot | undefined; maxParallelism: number; @@ -222,6 +223,14 @@ export class OperationExecutionRecord implements IOperationRunnerContext, IOpera return this._context.createEnvironment?.(this); } + /** + * Whether this operation's runner should remain resident after the operation completes for the + * current iteration. Defaults to `true` (kept warm) when the host supplies no persistence policy. + */ + public get shouldRunnerPersist(): boolean { + return this._context.shouldRunnerPersist?.(this.operation) ?? true; + } + public getInvalidateCallback(): (reason: string) => void { const invalidateFn: ((operations: Iterable, reason: string) => void) | undefined = this._context.invalidate; diff --git a/libraries/rush-lib/src/logic/operations/OperationGraph.ts b/libraries/rush-lib/src/logic/operations/OperationGraph.ts index 9ab09fe56a6..5cbfc254c43 100644 --- a/libraries/rush-lib/src/logic/operations/OperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/OperationGraph.ts @@ -85,7 +85,7 @@ interface IExecutionIterationContext extends IOperationExecutionRecordContext { promise: Promise | undefined; startTime?: number; - getRunnerPersistence: IOperationGraphIterationOptions['getRunnerPersistence']; + shouldRunnerPersist: IOperationGraphIterationOptions['shouldRunnerPersist']; completedOperations: number; totalOperations: number; @@ -576,12 +576,12 @@ export class OperationGraph implements IOperationGraph { const { startTime = performance.now(), inputsSnapshot = await getInputsSnapshotAsync?.(), - getRunnerPersistence + shouldRunnerPersist } = iterationOptions; const iterationOptionsForCallbacks: IOperationGraphIterationOptions = { startTime, inputsSnapshot, - getRunnerPersistence + shouldRunnerPersist }; const { hooks } = this; @@ -615,7 +615,7 @@ export class OperationGraph implements IOperationGraph { const iterationContext: IExecutionIterationContext = { abortController, startTime, - getRunnerPersistence, + shouldRunnerPersist, streamCollator, terminal, inputsSnapshot, @@ -767,7 +767,7 @@ export class OperationGraph implements IOperationGraph { const iterationOptions: IOperationGraphIterationOptions = { inputsSnapshot: iterationContext.inputsSnapshot, startTime: iterationContext.startTime, - getRunnerPersistence: iterationContext.getRunnerPersistence + shouldRunnerPersist: iterationContext.shouldRunnerPersist }; const executionQueue: AsyncOperationQueue = new AsyncOperationQueue( @@ -1021,11 +1021,14 @@ export class OperationGraph implements IOperationGraph { telemetry.log(logEntry); } - const { getRunnerPersistence } = iterationContext; - if (getRunnerPersistence) { + // IPC operations that executed this iteration release their runner immediately upon completion. + // This fallback closes cold runners that remain active because their operation did not execute, + // for example due to a cache hit, abort, blocked dependency, or disabled operation. + const { shouldRunnerPersist } = iterationContext; + if (shouldRunnerPersist) { const operationsToClose: Operation[] = []; for (const operation of this.operations) { - if (!getRunnerPersistence(operation)) { + if (!shouldRunnerPersist(operation) && operation.runner?.isActive !== false) { operationsToClose.push(operation); } } diff --git a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts index a4381421efc..50ee72ec07c 100644 --- a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts +++ b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts @@ -108,9 +108,23 @@ function createGraph( } class ClosableRunner extends MockOperationRunner { + public isActive: boolean = false; + public readonly closeAsync: jest.Mock, []> = jest.fn(async () => { - /* no-op */ + this.isActive = false; }); + + public override async executeAsync(context: IOperationRunnerContext): Promise { + this.isActive = true; + const status: OperationStatus = await super.executeAsync(context); + // Mirror IPCOperationRunner: a runner that owns a long-lived resource must release it + // immediately upon completion when the host has opted this operation out of persistence for + // the current iteration, rather than waiting until the end of the iteration. + if (context.shouldRunnerPersist === false) { + await this.closeAsync(); + } + return status; + } } describe('OperationGraph', () => { @@ -1194,19 +1208,107 @@ describe('runner persistence policy', () => { ); await graph.executeAsync({ - getRunnerPersistence: (operation) => operation !== oneShotOperation + shouldRunnerPersist: (operation) => operation !== oneShotOperation }); expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); expect(oneShotRunner.closeAsync).toHaveBeenCalledTimes(1); expect(changingRunner.closeAsync).not.toHaveBeenCalled(); await graph.executeAsync({ - getRunnerPersistence: (operation) => operation === persistentOperation + shouldRunnerPersist: (operation) => operation === persistentOperation }); expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); expect(oneShotRunner.closeAsync).toHaveBeenCalledTimes(2); expect(changingRunner.closeAsync).toHaveBeenCalledTimes(1); }); + + it('releases a non-persistent runner before its dependents execute', async () => { + const executionOrder: string[] = []; + + const upstreamRunner: ClosableRunner = new ClosableRunner('upstream', async () => { + executionOrder.push('upstream:run'); + return OperationStatus.Success; + }); + upstreamRunner.closeAsync.mockImplementation(async () => { + executionOrder.push('upstream:close'); + upstreamRunner.isActive = false; + }); + + const downstreamRunner: ClosableRunner = new ClosableRunner('downstream', async () => { + executionOrder.push('downstream:run'); + return OperationStatus.Success; + }); + + const upstreamOperation: Operation = new Operation({ + runner: upstreamRunner, + logFilenameIdentifier: 'upstream', + phase: mockPhase, + project: getOrCreateProject('upstream') + }); + const downstreamOperation: Operation = new Operation({ + runner: downstreamRunner, + logFilenameIdentifier: 'downstream', + phase: mockPhase, + project: getOrCreateProject('downstream') + }); + downstreamOperation.addDependency(upstreamOperation); + + const graph: OperationGraph = new OperationGraph( + new Set([upstreamOperation, downstreamOperation]), + graphOptions + ); + + await graph.executeAsync({ + shouldRunnerPersist: (operation) => operation !== upstreamOperation + }); + + expect(upstreamRunner.closeAsync).toHaveBeenCalledTimes(1); + expect(downstreamRunner.closeAsync).not.toHaveBeenCalled(); + // The non-persistent upstream runner must be released immediately upon its own completion, + // before the downstream operation begins executing — not deferred to the end of the iteration. + expect(executionOrder).toEqual(['upstream:run', 'upstream:close', 'downstream:run']); + }); + + it('closes an active cold runner when an iteration bypasses runner execution', async () => { + const persistentRunner: ClosableRunner = new ClosableRunner('persistent'); + const coldRunner: ClosableRunner = new ClosableRunner('cold'); + + const persistentOperation: Operation = new Operation({ + runner: persistentRunner, + logFilenameIdentifier: 'persistent', + phase: mockPhase, + project: getOrCreateProject('persistent') + }); + const coldOperation: Operation = new Operation({ + runner: coldRunner, + logFilenameIdentifier: 'cold', + phase: mockPhase, + project: getOrCreateProject('cold') + }); + + const graph: OperationGraph = new OperationGraph( + new Set([persistentOperation, coldOperation]), + graphOptions + ); + + await graph.executeAsync({}); + expect(persistentRunner.isActive).toBe(true); + expect(coldRunner.isActive).toBe(true); + + graph.hooks.beforeExecuteIterationAsync.tapPromise( + 'test', + async (): Promise => OperationStatus.FromCache + ); + + await graph.executeAsync({ + shouldRunnerPersist: (operation) => operation === persistentOperation + }); + + expect(coldRunner.closeAsync).toHaveBeenCalledTimes(1); + expect(coldRunner.isActive).toBe(false); + expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); + expect(persistentRunner.isActive).toBe(true); + }); }); describe('closeRunnersAsync', () => {