Compare commits

..

1 Commits

Author SHA1 Message Date
eric sciple
c5ea90fdcd Fix cancellation token race in EvaluateAndCompare 2026-03-06 12:14:33 -08:00
10 changed files with 78 additions and 34 deletions

View File

@@ -111,10 +111,10 @@ jobs:
core.setOutput('version', version);
- name: Setup Docker buildx
uses: docker/setup-buildx-action@v4
uses: docker/setup-buildx-action@v3
- name: Build Docker image
uses: docker/build-push-action@v7
uses: docker/build-push-action@v6
with:
context: ./images
load: true

View File

@@ -38,7 +38,7 @@ jobs:
core.setOutput('version', runnerVersion);
- name: Setup Docker buildx
uses: docker/setup-buildx-action@v4
uses: docker/setup-buildx-action@v3
- name: Log into registry ${{ env.REGISTRY }}
uses: docker/login-action@v4
@@ -49,7 +49,7 @@ jobs:
- name: Build and push Docker image
id: build-and-push
uses: docker/build-push-action@v7
uses: docker/build-push-action@v6
with:
context: ./images
platforms: |
@@ -68,7 +68,7 @@ jobs:
org.opencontainers.image.description=https://github.com/actions/runner/releases/tag/v${{ steps.image.outputs.version }}
- name: Generate attestation
uses: actions/attest-build-provenance@v4
uses: actions/attest-build-provenance@v3
with:
subject-name: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
subject-digest: ${{ steps.build-and-push.outputs.digest }}

View File

@@ -309,7 +309,7 @@ jobs:
core.setOutput('version', runnerVersion);
- name: Setup Docker buildx
uses: docker/setup-buildx-action@v4
uses: docker/setup-buildx-action@v3
- name: Log into registry ${{ env.REGISTRY }}
uses: docker/login-action@v4
@@ -320,7 +320,7 @@ jobs:
- name: Build and push Docker image
id: build-and-push
uses: docker/build-push-action@v7
uses: docker/build-push-action@v6
with:
context: ./images
platforms: |
@@ -339,7 +339,7 @@ jobs:
org.opencontainers.image.description=https://github.com/actions/runner/releases/tag/v${{ steps.image.outputs.version }}
- name: Generate attestation
uses: actions/attest-build-provenance@v4
uses: actions/attest-build-provenance@v3
with:
subject-name: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
subject-digest: ${{ steps.build-and-push.outputs.digest }}

View File

@@ -5,8 +5,8 @@ ARG TARGETOS
ARG TARGETARCH
ARG RUNNER_VERSION
ARG RUNNER_CONTAINER_HOOKS_VERSION=0.7.0
ARG DOCKER_VERSION=29.3.0
ARG BUILDX_VERSION=0.32.1
ARG DOCKER_VERSION=29.2.0
ARG BUILDX_VERSION=0.31.1
RUN apt update -y && apt install curl unzip -y

View File

@@ -12,7 +12,7 @@
"@actions/glob": "^0.4.0"
},
"devDependencies": {
"@stylistic/eslint-plugin": "^5.10.0",
"@stylistic/eslint-plugin": "^5.9.0",
"@types/node": "^22.0.0",
"@typescript-eslint/eslint-plugin": "^8.0.0",
"@typescript-eslint/parser": "^8.0.0",
@@ -228,9 +228,9 @@
}
},
"node_modules/@stylistic/eslint-plugin": {
"version": "5.10.0",
"resolved": "https://registry.npmjs.org/@stylistic/eslint-plugin/-/eslint-plugin-5.10.0.tgz",
"integrity": "sha512-nPK52ZHvot8Ju/0A4ucSX1dcPV2/1clx0kLcH5wDmrE4naKso7TUC/voUyU1O9OTKTrR6MYip6LP0ogEMQ9jPQ==",
"version": "5.9.0",
"resolved": "https://registry.npmjs.org/@stylistic/eslint-plugin/-/eslint-plugin-5.9.0.tgz",
"integrity": "sha512-FqqSkvDMYJReydrMhlugc71M76yLLQWNfmGq+SIlLa7N3kHp8Qq8i2PyWrVNAfjOyOIY+xv9XaaYwvVW7vroMA==",
"dev": true,
"dependencies": {
"@eslint-community/eslint-utils": "^4.9.1",
@@ -4854,9 +4854,9 @@
}
},
"@stylistic/eslint-plugin": {
"version": "5.10.0",
"resolved": "https://registry.npmjs.org/@stylistic/eslint-plugin/-/eslint-plugin-5.10.0.tgz",
"integrity": "sha512-nPK52ZHvot8Ju/0A4ucSX1dcPV2/1clx0kLcH5wDmrE4naKso7TUC/voUyU1O9OTKTrR6MYip6LP0ogEMQ9jPQ==",
"version": "5.9.0",
"resolved": "https://registry.npmjs.org/@stylistic/eslint-plugin/-/eslint-plugin-5.9.0.tgz",
"integrity": "sha512-FqqSkvDMYJReydrMhlugc71M76yLLQWNfmGq+SIlLa7N3kHp8Qq8i2PyWrVNAfjOyOIY+xv9XaaYwvVW7vroMA==",
"dev": true,
"requires": {
"@eslint-community/eslint-utils": "^4.9.1",

View File

@@ -35,7 +35,7 @@
"@actions/glob": "^0.4.0"
},
"devDependencies": {
"@stylistic/eslint-plugin": "^5.10.0",
"@stylistic/eslint-plugin": "^5.9.0",
"@types/node": "^22.0.0",
"@typescript-eslint/eslint-plugin": "^8.0.0",
"@typescript-eslint/parser": "^8.0.0",

View File

@@ -15,7 +15,7 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="System.Text.Encoding.CodePages" Version="10.0.3" />
<PackageReference Include="System.Text.Encoding.CodePages" Version="8.0.0" />
<PackageReference Include="Microsoft.Win32.Registry" Version="5.0.0" />
<PackageReference Include="System.Threading.Channels" Version="8.0.0" />
</ItemGroup>

View File

@@ -77,8 +77,7 @@ namespace GitHub.Runner.Worker
List<string> StepEnvironmentOverrides { get; }
ExecutionContext Root { get; }
ExecutionContext Parent { get; }
IExecutionContext Root { get; }
// Initialize
void InitializeJob(Pipelines.AgentJobRequestMessage message, CancellationToken token);
@@ -251,7 +250,9 @@ namespace GitHub.Runner.Worker
}
}
public ExecutionContext Root
IExecutionContext IExecutionContext.Root => Root;
private ExecutionContext Root
{
get
{
@@ -266,13 +267,7 @@ namespace GitHub.Runner.Worker
}
}
public ExecutionContext Parent
{
get
{
return _parentExecutionContext;
}
}
public JobContext JobContext
{

View File

@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Threading;
using GitHub.Actions.WorkflowParser;
using GitHub.DistributedTask.Expressions2;
using GitHub.DistributedTask.ObjectTemplating.Tokens;
@@ -226,8 +227,12 @@ namespace GitHub.Runner.Worker
Func<TNew> newEvaluator,
Func<TLegacy, TNew, bool> resultComparer)
{
// Capture cancellation state before evaluation
var cancellationRequestedBefore = _context.CancellationToken.IsCancellationRequested;
// Use the root (job-level) cancellation token to detect cancellation race conditions.
// The step-level token only fires on step timeout, not on job cancellation.
// Job cancellation mutates JobContext.Status which expression functions read,
// so we need the root token to properly detect cancellation between evaluator runs.
var rootCancellationToken = _context.Root?.CancellationToken ?? CancellationToken.None;
var cancellationRequestedBefore = rootCancellationToken.IsCancellationRequested;
// Legacy evaluator
var legacyException = default(Exception);
@@ -261,7 +266,7 @@ namespace GitHub.Runner.Worker
}
// Capture cancellation state after evaluation
var cancellationRequestedAfter = _context.CancellationToken.IsCancellationRequested;
var cancellationRequestedAfter = rootCancellationToken.IsCancellationRequested;
// Compare results or exceptions
bool hasMismatch = false;

View File

@@ -19,6 +19,7 @@ namespace GitHub.Runner.Common.Tests.Worker
public sealed class PipelineTemplateEvaluatorWrapperL0
{
private CancellationTokenSource _ecTokenSource;
private CancellationTokenSource _rootTokenSource;
private Mock<IExecutionContext> _ec;
private TestHostContext _hc;
@@ -65,7 +66,7 @@ namespace GitHub.Runner.Common.Tests.Worker
var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object, allowServiceContainerCommand: false);
// Call EvaluateAndCompare directly: the new evaluator cancels the token
// Call EvaluateAndCompare directly: the new evaluator cancels the root token
// and returns a different value, forcing hasMismatch = true.
// Because cancellation flipped during the evaluation window, the
// mismatch should be skipped.
@@ -74,7 +75,7 @@ namespace GitHub.Runner.Common.Tests.Worker
() => "legacy-value",
() =>
{
_ecTokenSource.Cancel();
_rootTokenSource.Cancel();
return "different-value";
},
(legacy, @new) => string.Equals(legacy, @new, StringComparison.Ordinal));
@@ -88,6 +89,43 @@ namespace GitHub.Runner.Common.Tests.Worker
}
}
[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public void EvaluateAndCompare_SkipsMismatchRecording_WhenRootCancellationOccursBetweenEvaluators()
{
// Simulates job-level cancellation firing between legacy and new evaluator runs.
// Root is mocked with a separate CancellationTokenSource to exercise the
// _context.Root?.CancellationToken path (the job-level token).
try
{
Setup();
_ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true");
var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object, allowServiceContainerCommand: false);
// Legacy evaluator cancels the root token (simulating job cancel) and returns a value.
// The new evaluator returns a different value. The mismatch should be skipped.
var result = wrapper.EvaluateAndCompare<string, string>(
"TestRootCancellationSkip",
() =>
{
var legacyValue = "legacy-value";
_rootTokenSource.Cancel();
return legacyValue;
},
() => "different-value",
(legacy, @new) => string.Equals(legacy, @new, StringComparison.Ordinal));
Assert.Equal("legacy-value", result);
Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch);
}
finally
{
Teardown();
}
}
[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
@@ -862,6 +900,8 @@ namespace GitHub.Runner.Common.Tests.Worker
{
_ecTokenSource?.Dispose();
_ecTokenSource = new CancellationTokenSource();
_rootTokenSource?.Dispose();
_rootTokenSource = new CancellationTokenSource();
_hc = new TestHostContext(this, name);
@@ -877,6 +917,9 @@ namespace GitHub.Runner.Common.Tests.Worker
WriteDebug = true,
});
_ec.Setup(x => x.CancellationToken).Returns(_ecTokenSource.Token);
var rootEc = new Mock<IExecutionContext>();
rootEc.Setup(x => x.CancellationToken).Returns(_rootTokenSource.Token);
_ec.Setup(x => x.Root).Returns(rootEc.Object);
_ec.Setup(x => x.ExpressionValues).Returns(expressionValues);
_ec.Setup(x => x.ExpressionFunctions).Returns(expressionFunctions);
_ec.Setup(x => x.Write(It.IsAny<string>(), It.IsAny<string>())).Callback((string tag, string message) => { _hc.GetTrace().Info($"{tag}{message}"); });
@@ -885,6 +928,7 @@ namespace GitHub.Runner.Common.Tests.Worker
private void Teardown()
{
_rootTokenSource?.Dispose();
_hc?.Dispose();
}
}