Apply .cctor deadlock defense on .NET Core (follow-up to #8498)#8517
Merged
Conversation
NachoEchevarria
changed the title
BenchmarksBenchmark execution time: 2026-04-24 11:42:20 Comparing candidate commit 04042b4 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 0 unstable metrics, 61 known flaky benchmarks, 26 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8517) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8517) - mean (73ms) : 70, 75
master - mean (73ms) : 70, 77
section Bailout
This PR (8517) - mean (77ms) : 75, 78
master - mean (79ms) : 74, 83
section CallTarget+Inlining+NGEN
This PR (8517) - mean (1,087ms) : 1044, 1131
master - mean (1,077ms) : 1027, 1127
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8517) - mean (115ms) : 110, 120
master - mean (118ms) : 112, 124
section Bailout
This PR (8517) - mean (116ms) : 114, 119
master - mean (117ms) : 113, 121
section CallTarget+Inlining+NGEN
This PR (8517) - mean (782ms) : 753, 811
master - mean (780ms) : 752, 808
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8517) - mean (101ms) : 97, 104
master - mean (104ms) : 99, 109
section Bailout
This PR (8517) - mean (106ms) : 100, 112
master - mean (101ms) : 98, 104
section CallTarget+Inlining+NGEN
This PR (8517) - mean (946ms) : 906, 987
master - mean (939ms) : 909, 969
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8517) - mean (101ms) : 96, 106
master - mean (102ms) : 97, 108
section Bailout
This PR (8517) - mean (101ms) : 98, 104
master - mean (104ms) : 99, 108
section CallTarget+Inlining+NGEN
This PR (8517) - mean (830ms) : 786, 874
master - mean (816ms) : 780, 852
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8517) - mean (204ms) : 194, 214
master - mean (202ms) : 194, 210
section Bailout
This PR (8517) - mean (208ms) : 198, 219
master - mean (207ms) : 197, 217
section CallTarget+Inlining+NGEN
This PR (8517) - mean (1,209ms) : 1149, 1269
master - mean (1,199ms) : 1143, 1255
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8517) - mean (293ms) : 280, 305
master - mean (289ms) : 280, 299
section Bailout
This PR (8517) - mean (294ms) : 281, 307
master - mean (290ms) : 279, 301
section CallTarget+Inlining+NGEN
This PR (8517) - mean (970ms) : 944, 995
master - mean (954ms) : 921, 987
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8517) - mean (288ms) : 272, 303
master - mean (284ms) : 274, 294
section Bailout
This PR (8517) - mean (288ms) : 272, 303
master - mean (282ms) : 270, 295
section CallTarget+Inlining+NGEN
This PR (8517) - mean (1,164ms) : 1115, 1213
master - mean (1,153ms) : 1114, 1191
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8517) - mean (287ms) : 274, 300
master - mean (283ms) : 270, 297
section Bailout
This PR (8517) - mean (287ms) : 275, 299
master - mean (281ms) : 271, 291
section CallTarget+Inlining+NGEN
This PR (8517) - mean (1,060ms) : 978, 1143
master - mean (1,039ms) : 992, 1085
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
NachoEchevarria
added
the
area:tracer
NachoEchevarria
marked this pull request as ready for review
April 24, 2026 12:15
andrewlock
approved these changes
Apr 24, 2026
| StartupLogger.Debug("Total number of assemblies: {0}", _assemblies.Length); | ||
| } | ||
|
|
||
| internal static Assembly? OnAssemblyResolve(object sender, ResolveEventArgs args) |
Member
There was a problem hiding this comment.
I believe this is just a cut-and paste from Startup.NetCore, right?
Collaborator
Author
There was a problem hiding this comment.
Almost. It was, but I noticed that the original code had no try/catch (the .net framework version does) so these were added.
zacharycmontoya
approved these changes
Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #8498. No known .NET Core trigger today; defense-in-depth for consistency.
Summary of changes
Mirror the .NET Framework fix from #8498 on .NET Core. Move the
AppDomain.AssemblyResolveandAssemblyLoadContext.Default.Resolvinghandlers, plus the assembly cache andDependencyLoadContext, offStartupontoManagedProfilerAssemblyResolver.Reason for change
The mechanism behind #8498 — "invoking a static member requires the declaring type's
.cctorto have completed" — is runtime-generic; only the trigger (<appSettings>configBuilder) was Framework-specific. On .NET Core bothAppDomain.AssemblyResolve(static method onStartup) andAssemblyLoadContext.Default.Resolving(lambda compiled to a static method onStartup) have the same latent hazard if anything in the.cctorchain ever does sync-over-async.Implementation details
Same shape as #8498:
ManagedProfilerAssemblyResolver.NetCore.cswith the handlers,ResolveAssembly,DependencyLoadContext, cache, and aPopulateAssemblyCacheentry point.Startup.NetCore.csslimmed —ComputeTfmDirectorydelegates cache population to the resolver.Startup.cs:#if NETFRAMEWORKconditionals removed; both TFMs subscribe through the resolver, and the ALCResolvinglambda now points atOnAssemblyLoadContextResolving.LoadAssemblyfallback uses the resolver unconditionally.Both
OnAssemblyResolveandOnAssemblyLoadContextResolvingwrapResolveAssemblyin atry/catchthat logs viaStartupLoggerand returnsnull, matching the .NET Framework resolver. Without this, a throw fromLoadFromAssemblyPath(e.g.BadImageFormatException,FileLoadException) would propagate into whoever triggered the resolve — often a credential-chain probe expectingnullon miss — breaking their fallback logic.Startup..cctorseedsManagedProfilerAssemblyResolver.ManagedProfilerDirectorybefore subscribing, triggering the resolver's trivial.cctorup-front so ThreadPool dispatches don't wait on anything.Test coverage
No new tests — the deadlock is theoretical on .NET Core. Existing loader/instrumentation tests exercise the new dispatch path.
Other details
None.