Uh oh!
There was an error while loading. Please reload this page.
Add thread-safety locks to StepsContext - #4475
Conversation
6a9dd1b to
057432eCompareThere was a problem hiding this comment.
Pull request overview
This PR prepares the runner for future concurrent/background step execution by extending step/timeline wire contracts with background-step metadata and introducing synchronization around some shared mutable state (e.g., telemetry/step context).
Changes:
- Extend Results/RSWebApi/DTWebApi step & timeline contracts to carry background-step metadata (e.g.,
is_background, control type, control step IDs, parallel group). - Introduce a new pipeline step type (
BackgroundStepControl) and update step deserialization to recognize it. - Add locking around
StepsContextmutations and someGlobalContextcollection mutations to reduce concurrency hazards.
Show a summary per file
| File | Description |
|---|---|
| src/Sdk/WebApi/WebApi/ResultsHttpClient.cs | Populates new background-step fields when converting timeline records to Results “Step” payloads. |
| src/Sdk/WebApi/WebApi/Contracts.cs | Adds background-step fields to Results Step contract. |
| src/Sdk/RSWebApi/Contracts/StepResult.cs | Adds background-step fields to RSWebApi step result contract. |
| src/Sdk/DTWebApi/WebApi/TimelineRecord.cs | Adds background-step fields to timeline records and copies them during clone. |
| src/Sdk/DTPipelines/Pipelines/StepConverter.cs | Updates step JSON deserialization to instantiate BackgroundStepControl. |
| src/Sdk/DTPipelines/Pipelines/Step.cs | Registers new step type via KnownType and enum value. |
| src/Sdk/DTPipelines/Pipelines/JobStep.cs | Adds/clones ParallelGroupId for job steps. |
| src/Sdk/DTPipelines/Pipelines/BackgroundStepControl.cs | Introduces new pipeline step type for background control flow (wait/wait-all/cancel). |
| src/Sdk/DTPipelines/Pipelines/ActionStep.cs | Adds/clones Background flag on action steps. |
| src/Runner.Worker/StepsContext.cs | Adds a lock around several StepsContext mutation paths. |
| src/Runner.Worker/PipelineTemplateEvaluatorWrapper.cs | Locks around telemetry list writes for mismatch reporting. |
| src/Runner.Worker/Handlers/HandlerFactory.cs | Locks around global HashSet mutations that track Node migration telemetry. |
| src/Runner.Worker/GlobalContext.cs | Adds a shared lock object intended for guarding global collections. |
| src/Runner.Worker/ActionManifestManagerWrapper.cs | Locks around telemetry list writes for mismatch reporting. |
| src/Runner.Worker/ActionManager.cs | Locks around JobTelemetry additions in action download path. |
| src/Runner.Worker/ActionCommandManager.cs | Locks around JobTelemetry additions for action command telemetry. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 6
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
becd2c1 to
b88b481Compareb88b481 to
5007995CompareUh oh!
There was an error while loading. Please reload this page.
Summary
Adds thread-safety primitives to shared mutable state in the runner worker, preparing for concurrent step execution in background steps.
Why
Today, steps execute sequentially so these collections are never accessed concurrently. Background steps will allow multiple steps to run in parallel within a single job, meaning these shared collections can be mutated from multiple threads simultaneously. Adding locks now is a safe, no-op change for sequential execution and unblocks the background steps feature.