fix: prevent sync-over-async deadlock when async configure callback is used in TypeScript AppHost#26
Merged
kieronlanning merged 4 commits intoMay 27, 2026
Conversation
The configure delegate passed to AddAspireC4 is an ATS-generated proxy for a TypeScript async callback. The proxy calls .GetAwaiter().GetResult() internally to bridge the sync/async boundary. Previously, configure?.Invoke(opts) was called inside the lazy IOptions.Configure callback. When DistributedApplication.RunAsync accessed IOptions<AspireC4DiagramOptions>.Value, that callback ran on StreamJsonRpc's NonConcurrentSynchronizationContext. The .GetResult() call blocked the context while waiting for TypeScript's incoming setter responses, which themselves needed the same blocked context to be dispatched. Classic sync-over-async deadlock. Fix: eagerly evaluate configure on the background thread that AddAspireC4 runs on (guaranteed by RunSyncOnBackgroundThread = true). Configuration values are bound first (so the callback sees and can intentionally override/clear them), then configure?.Invoke is called, and the resulting fully-materialised snapshot is captured. The lazy IOptions.Configure callback uses CopyTo to apply that snapshot, never touching the ATS proxy. Add AspireC4DiagramOptions.CopyTo(target) - copies all properties including collection and dictionary types without sharing mutable references. Add regression tests covering: - CopyTo scalar, nullable, collection, and dictionary properties - CopyTo preserving explicit null/empty overrides (code wins over defaults) - IOptions resolution applying configure callback values end-to-end
…callback deadlock Add two structured log entries to IAspireC4LifecycleHookTelemetry: - ApplyingDiagramOptionsSnapshot (Debug) — emitted immediately before IOptions.Value is resolved so a deadlock in this area leaves a clear breadcrumb in the log - DiagramOptionsSnapshotApplied (Debug) — emitted after successful resolution with key option values for traceability Add a Troubleshooting section to the README covering the sync-over-async deadlock that caused aspire start to hang silently for 60 s. Includes symptom, cause, dotnet-dump diagnosis steps, and the upstream issue reference (microsoft/aspire#17487).
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR addresses a TypeScript AppHost deadlock scenario caused by invoking an ATS-proxied async configure callback inside a lazy IOptions.Configure pipeline, and adds regression coverage plus diagnostics to make future hangs easier to identify.
Changes:
- Eagerly materialize
AspireC4DiagramOptions(defaults → config → callback) and apply via a newCopyTomethod in the lazy options pipeline. - Add unit/regression tests validating
IOptionsapplication and copy semantics (including explicit null overrides and collection replacement). - Add lifecycle telemetry and documentation to aid diagnosing 60s
aspire starttimeouts caused by sync-over-async deadlocks.
Show a summary per file
| File | Description |
|---|---|
| src/src/AspireC4/Extensions/Aspire/Hosting/AspireC4DistributedApplicationBuilderExtensions.cs | Eagerly evaluates configuration + callback and applies a snapshot via CopyTo to avoid ATS deadlock. |
| src/src/AspireC4/AspireC4DiagramOptions.cs | Adds CopyTo to safely apply a fully-materialized options snapshot without invoking ATS callbacks in lazy configure. |
| src/src/AspireC4/Lifecycle/AspireC4LifecycleHook.ContainerBindMount.cs | Adds debug telemetry breadcrumbs around options resolution to help diagnose future hangs. |
| src/src/AspireC4/Lifecycle/IAspireC4LifecycleHookTelemetry.cs | Extends telemetry surface with debug events for options snapshot application. |
| src/tests/AspireC4.UnitTests/...AspireC4DistributedApplicationBuilderExtensionsTests.cs | Adds regression tests ensuring callback values are applied via IOptions including explicit null overrides. |
| src/tests/AspireC4.UnitTests/AspireC4DiagramOptionsTests.cs | Adds tests for CopyTo behavior across scalars, nullables, collections, and dictionaries. |
| README.md | Documents the 60s hang symptom, root cause, diagnosis steps, and the fix strategy. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 4
Comment on lines
70
to
83
| AspireC4DiagramOptions diagramOpts = new(); | ||
| builder.Configuration.Bind(AspireC4DiagramOptions.SectionName, diagramOpts); | ||
| configure?.Invoke(diagramOpts); | ||
|
|
||
| builder | ||
| .Services.AddOptions<AspireC4DiagramOptions>() | ||
| .BindConfiguration(AspireC4DiagramOptions.SectionName) | ||
| .Configure(opts => | ||
| { | ||
| configure?.Invoke(opts); | ||
| // Apply the eagerly-computed snapshot. See the comment above for why we do this | ||
| // instead of calling configure?.Invoke(opts) directly. | ||
| diagramOpts.CopyTo(opts); | ||
| opts.OutputDirectory = ResolveOutputDirectory(builder.AppHostDirectory, opts.OutputDirectory); | ||
| }); |
Comment on lines
+424
to
+427
| // Dictionary properties — new instances so callers can't mutate shared state. | ||
| target.ImageAliases = new Dictionary<string, string>(ImageAliases, StringComparer.OrdinalIgnoreCase); | ||
| target.StateTagMap = new Dictionary<string, string?>(StateTagMap); | ||
| target.ConfigFileMetadata = new Dictionary<string, string>(ConfigFileMetadata); |
Comment on lines
+17
to
+21
| telemetry.ApplyingDiagramOptionsSnapshot(); | ||
|
|
||
| var opts = options.Value; | ||
|
|
||
| telemetry.DiagramOptionsSnapshotApplied(opts.OutputDirectory, opts.FormatGeneratedFile, opts.DisableHMR); |
Comment on lines
+55
to
+56
| // Eagerly evaluate `configure` here — on the background thread that AddAspireC4 runs on | ||
| // (guaranteed by RunSyncOnBackgroundThread = true). Doing so is safe because: |
- ApplyDelta: evaluate callback against fresh defaults (no config binding), restore BindConfiguration lazily, apply only callback-changed properties on top of config — preserves late-added configuration values and correct config < code precedence (microsoft/aspire#17487) - CopyTo/ApplyDelta: preserve source dictionary comparer instead of hardcoding OrdinalIgnoreCase for ImageAliases - Telemetry: log only the output directory name (not the full path) to avoid leaking sensitive filesystem paths in logs - Comment: scope the RunSyncOnBackgroundThread guarantee to the ATS/TypeScript export path; C# callers are not affected - Tests: add AddAspireC4_IOptions_LateAddedConfigIsReflected, AddAspireC4_IOptions_CallbackWinsOverConfig, and six ApplyDelta unit tests covering the scalar/collection/nullable/ExcludedResourceTypes delta semantics
Merged
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
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.
Fixes a deadlock that caused
aspire start/aspire runto hang silently for 60 seconds when anasyncconfigure callback is passed toAddAspireC4from a TypeScript AppHost.Root cause
The ATS proxy for a TypeScript
asynccallback calls.GetAwaiter().GetResult()internally. WhenIOptions<AspireC4DiagramOptions>.Valuewas resolved lazily during aBeforeStartEventsubscriber, theIOptions.Configurecallback invoked this proxy, blocking StreamJsonRpc'sNonConcurrentSynchronizationContextwhile waiting for TypeScript setter responses that were themselves queued on the same blocked context.Upstream framework issue: microsoft/aspire#17487.
Fix
The configure callback is now evaluated eagerly while
AddAspireC4is executing, before startup enters the lazyIOptionsresolution path that can run on the non-concurrent sync context.To preserve the existing options precedence (
defaults < config < code) without reintroducing the deadlock, the PR captures the callback's changes against fresh defaults and applies only that delta afterBindConfigurationruns lazily. This keeps late-added configuration working while still ensuring callback-authored values win over config-bound values.Longer-term API direction
This PR keeps the existing
Action<AspireC4DiagramOptions>API compatible, but the cleaner TypeScript-facing design is a single options object / patch DTO instead of an async callback:That would let the implementation apply an explicit patch after config binding:
This avoids both problems: no ATS callback has to be synchronously bridged during options resolution, and the implementation no longer has to infer intent by diffing a mutated options instance against defaults.
Review feedback addressed
BindConfigurationso configuration added afterAddAspireC4is still reflected.RunSyncOnBackgroundThreadguarantee is scoped to the ATS/TypeScript export path.Tests
Added coverage for:
IOptions<AspireC4DiagramOptions>.ExcludedResourceTypesoptions.Validation:
dotnet test src\AspireC4.slnxpassed with 812 tests.