Fix .NET Framework startup deadlock from configBuilder sync-over-async#8498
Merged
Conversation
NachoEchevarria
changed the title
Investigation-only notes with workstation-specific paths; PR description carries the relevant context.
NachoEchevarria
added
type:bug
area:tracer
BenchmarksBenchmark execution time: 2026-04-22 15:15:02 Comparing candidate commit 862f3a0 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 0 unstable metrics, 60 known flaky benchmarks, 27 flaky benchmarks without significant changes.
|
NachoEchevarria
marked this pull request as ready for review
April 23, 2026 08:23
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8498) 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 (8498) - mean (74ms) : 71, 78
master - mean (75ms) : 71, 79
section Bailout
This PR (8498) - mean (77ms) : 75, 79
master - mean (79ms) : 75, 83
section CallTarget+Inlining+NGEN
This PR (8498) - mean (1,082ms) : 1037, 1127
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 (8498) - mean (113ms) : 110, 117
master - mean (118ms) : 112, 123
section Bailout
This PR (8498) - mean (118ms) : 111, 125
master - mean (115ms) : 112, 119
section CallTarget+Inlining+NGEN
This PR (8498) - mean (777ms) : 757, 798
master - mean (776ms) : 751, 801
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8498) - mean (103ms) : 97, 110
master - mean (102ms) : 96, 108
section Bailout
This PR (8498) - mean (101ms) : 98, 104
master - mean (101ms) : 98, 104
section CallTarget+Inlining+NGEN
This PR (8498) - mean (936ms) : 899, 974
master - mean (941ms) : 899, 983
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8498) - mean (100ms) : 95, 105
master - mean (101ms) : 97, 105
section Bailout
This PR (8498) - mean (104ms) : 99, 109
master - mean (104ms) : 99, 109
section CallTarget+Inlining+NGEN
This PR (8498) - mean (821ms) : 789, 852
master - mean (822ms) : 778, 867
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 (8498) - mean (201ms) : 192, 211
master - mean (206ms) : 198, 215
section Bailout
This PR (8498) - mean (205ms) : 197, 212
master - mean (208ms) : 202, 214
section CallTarget+Inlining+NGEN
This PR (8498) - mean (1,183ms) : 1128, 1238
master - mean (1,202ms) : 1145, 1259
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 (8498) - mean (289ms) : 278, 300
master - mean (293ms) : 283, 302
section Bailout
This PR (8498) - mean (290ms) : 279, 301
master - mean (295ms) : 283, 307
section CallTarget+Inlining+NGEN
This PR (8498) - mean (944ms) : 909, 979
master - mean (961ms) : 930, 992
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8498) - mean (283ms) : 270, 295
master - mean (286ms) : 275, 296
section Bailout
This PR (8498) - mean (282ms) : 274, 290
master - mean (287ms) : 277, 297
section CallTarget+Inlining+NGEN
This PR (8498) - mean (1,150ms) : 1107, 1194
master - mean (1,156ms) : 1115, 1196
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8498) - mean (281ms) : 273, 290
master - mean (285ms) : 273, 297
section Bailout
This PR (8498) - mean (282ms) : 270, 293
master - mean (286ms) : 275, 297
section CallTarget+Inlining+NGEN
This PR (8498) - mean (1,038ms) : 985, 1091
master - mean (1,049ms) : 973, 1126
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
andrewlock
approved these changes
Apr 23, 2026
andrewlock
left a comment
andrewlock
left a comment
Member
There was a problem hiding this comment.
Just to clarify:
- Previously the problem was that the
AssemblyResolvewould fire as part of the app.config loading, but would be blocked because the tracer initializes here, in theStartupstatic constructor. As the ctor isn't finished, theAssemblyResolvehandler can't run. - With the change, we're still initializing the tracer in the same place, and we're still running the same assembly resolve code, the only difference is that because the handler isn't defined on
Startup, the handler can run, so no deadlock
This seems like a problem that could theoretically occur in .NET Core too (although it won't be exactly the same because we don't load app.config). Is there any reason we shouldn't do the same thing for .NET Core as we did for .NET Framework, for consistency and safety? 🤔
Collaborator
Author
Agreed the same pattern should apply on .NET Core to avoid similar deadlocks. I can add that to a subsequent PR, if that's ok, to keep this PR focused on the escalation resolution. |
NachoEchevarria
added a commit
that referenced
this pull request
Apr 27, 2026
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.AssemblyResolve` and `AssemblyLoadContext.Default.Resolving` handlers, plus the assembly cache and `DependencyLoadContext`, off `Startup` onto `ManagedProfilerAssemblyResolver`. ## Reason for change The mechanism behind #8498 — "invoking a static member requires the declaring type's `.cctor` to have completed" — is runtime-generic; only the trigger (`<appSettings>` configBuilder) was Framework-specific. On .NET Core both `AppDomain.AssemblyResolve` (static method on `Startup`) and `AssemblyLoadContext.Default.Resolving` (lambda compiled to a static method on `Startup`) have the same latent hazard if anything in the `.cctor` chain ever does sync-over-async. ## Implementation details Same shape as #8498: - New `ManagedProfilerAssemblyResolver.NetCore.cs` with the handlers, `ResolveAssembly`, `DependencyLoadContext`, cache, and a `PopulateAssemblyCache` entry point. - `Startup.NetCore.cs` slimmed — `ComputeTfmDirectory` delegates cache population to the resolver. - `Startup.cs`: `#if NETFRAMEWORK` conditionals removed; both TFMs subscribe through the resolver, and the ALC `Resolving` lambda now points at `OnAssemblyLoadContextResolving`. `LoadAssembly` fallback uses the resolver unconditionally. Both `OnAssemblyResolve` and `OnAssemblyLoadContextResolving` wrap `ResolveAssembly` in a `try/catch` that logs via `StartupLogger` and returns `null`, matching the .NET Framework resolver. Without this, a throw from `LoadFromAssemblyPath` (e.g. `BadImageFormatException`, `FileLoadException`) would propagate into whoever triggered the resolve — often a credential-chain probe expecting `null` on miss — breaking their fallback logic. `Startup..cctor` seeds `ManagedProfilerAssemblyResolver.ManagedProfilerDirectory` before subscribing, triggering the resolver's trivial `.cctor` up-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.
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.
Summary of changes
Move the
AssemblyResolvehandler offStartuponto a sibling classManagedProfilerAssemblyResolver. .NET Framework only.Reason for change
A customer is reporting that a console application is getting hung. They provided the console app code and a memory dump.
Affected: .NET Framework console apps / Windows services whose
app.confighas a<appSettings>configBuilder that performs sync-over-async work (concrete trigger:AzureAppConfigurationBuilderwithDefaultAzureCredentialanduseAzureKeyVault="true").iThe process deadlocks before
Main()runs:Startup..cctor→ readsConfigurationManager.AppSettings→ the configBuilder callsTask.Wait. Main thread blocked.Type.GetType(...)during credential-chain probing, firingAppDomain.AssemblyResolve.Startup, the CLR requiresStartup..cctorto have completed before running it — it hasn't. The ThreadPool thread blocks onStartup..cctor; the main thread blocks on the Task. Classic.cctor× sync-over-async deadlock.Any static member on
Startupinvoked from a ThreadPool thread duringStartup..cctorhas the same hazard, so the structural fix is to move the handler offStartuprather than try to remove specific .cctor-time work.Implementation details
ManagedProfilerAssemblyResolveris a sibling class with a trivial.cctor.Startup..cctorseeds itsManagedProfilerDirectoryproperty and subscribesOnAssemblyResolvebefore any ThreadPool work can be scheduled; the resolver is fully initialized by the time a ThreadPool thread dispatches the handler, so dispatch doesn't wait on anything.Startup.AssemblyNamewas widened fromprivate consttointernal constso the sibling class can reference it. Const strings are inlined at compile time, so this is not a runtime type-init dependency. This change does not affect the automatic version bump mechanisms..NET Core paths are unchanged —
configBuildersis a .NET Framework-only feature, so the equivalent trigger doesn't exist there.Test coverage
Verified against a local repro mirroring the customer scenario (configBuilder with
DefaultAzureCredential+useAzureKeyVault+ mock Azure App Config returning a Key Vault reference):Starting Main().Starting Main()and continues normally.Customer-provided memory dump stacks match the pre-fix local repro stacks, confirming it's the same deadlock.
Other details