Synchronize access to System.ComponentModel.DataAnnotations.Validator

[kmcclellan]

Pull Request

The issue or feature being addressed

[Bug]: InvalidCastException when multiple threads retrieve pipeline

Details on the issue fix or feature implementation

.NET team has targeted a fix in v10 of the runtime. In the meantime, I've opted for a simple workaround that synchronizes validation calls for all strategy options models (these are the only ones that use RangeAttribute). Given this only occurs when a pipeline is first built, I don't feel like we need to be concerned about contention.

Confirm the following

• I started this PR by branching from the head of the default branch
• I have targeted the PR to merge into the default branch
• I have included unit tests for the issue/feature
• I have successfully run a local build

[kmcclellan]

@dotnet-policy-service agree

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.39%. Comparing base (a2a3dff) to head (2c1e5ba).
Report is 11 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2428   +/-   ##
=======================================
  Coverage   85.39%   85.39%           
=======================================
  Files         312      312           
  Lines        7464     7464           
  Branches     1121     1121           
=======================================
  Hits         6374     6374           
  Misses        905      905           
  Partials      185      185           

Flag	Coverage Δ
linux	85.36% <ø> (-0.03%)	⬇️
macos	85.36% <ø> (-0.03%)	⬇️
windows	85.33% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[martincostello]

It would be good to quantify what, if any, impact this has on performance by running the benchmarks before and after the change to see if there's any impact from introducing the lock (I'd hope not as it's on construction not execution).

[martincostello]

I ran the benchmarks for main and then this PR on my laptop to get a consistent baseline to compare (diff) and as expected there's no discernible difference, it's just noise.

Once some CI-related infrastructure changes are sorted out and merged, then I'll merge this 👍

[martincostello]

That change has been made now - you just need to rebase/merge with main.