Skip to content

Commit 3ac074b

Browse files
davidfowldavidfowl
authored
fix: prevent sync-over-async deadlock when async configure callback is used in TypeScript AppHost (#26)
* fix: prevent sync-over-async deadlock in IOptions configure callback 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 * docs: add troubleshooting guide and diagnostic logging for configure 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). * revert: remove README troubleshooting section * fix: address review feedback on deadlock fix - 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
1 parent 392c8e6 commit 3ac074b

6 files changed

Lines changed: 533 additions & 13 deletions

File tree

‎src/src/AspireC4/AspireC4DiagramOptions.cs‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,4 +354,171 @@ public sealed class AspireC4DiagramOptions
354354
/// </summary>
355355
[System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2227:Collection properties should be read only")]
356356
public Dictionary<string, string> ConfigFileMetadata { get; set; } = [];
357+
358+
/// <summary>
359+
/// Copies all property values from this instance to <paramref name="target"/>.
360+
/// </summary>
361+
/// <remarks>
362+
/// <para>
363+
/// This method is called inside the lazy <c>IOptions.Configure</c> callback registered by
364+
/// <c>AddAspireC4</c>, where we must NOT invoke the user's <c>configure</c> delegate directly.
365+
/// </para>
366+
/// <para>
367+
/// <b>Why we can't call <c>configure?.Invoke(opts)</c> lazily:</b> in the polyglot (TypeScript)
368+
/// AppHost scenario, <c>configure</c> is an ATS-generated proxy for an <c>async</c> TypeScript
369+
/// callback (e.g. <c>async (opts) =&gt; { await opts.title.set("…"); }</c>). That proxy calls
370+
/// <c>InvokeAsync(…).GetAwaiter().GetResult()</c> internally to bridge the async boundary.
371+
/// When <c>IOptions.Value</c> is accessed during <c>DistributedApplication.RunAsync</c>, the
372+
/// <c>Configure</c> callback executes on StreamJsonRpc's
373+
/// <c>NonConcurrentSynchronizationContext</c> (a single-item dispatch queue). The
374+
/// <c>.GetResult()</c> call blocks that context while waiting for TypeScript's incoming
375+
/// setter calls — which themselves need to be dispatched on the same blocked context.
376+
/// Classic sync-over-async deadlock.
377+
/// </para>
378+
/// <para>
379+
/// The fix: <c>AddAspireC4</c> eagerly invokes <c>configure</c> on a background thread
380+
/// (safe because <c>RunSyncOnBackgroundThread = true</c>), captures the fully-materialised
381+
/// options snapshot (defaults → config binding → user callback), and then uses
382+
/// <c>CopyTo</c> here to apply that snapshot in the lazy callback — no ATS proxy involved.
383+
/// </para>
384+
/// </remarks>
385+
internal void CopyTo(AspireC4DiagramOptions target)
386+
{
387+
// Scalar / simple-value properties
388+
target.GeneratedViewId = GeneratedViewId;
389+
target.DefaultViewId = DefaultViewId;
390+
target.Title = Title;
391+
target.ViewTitle = ViewTitle;
392+
target.ViewDescription = ViewDescription;
393+
target.OutputDirectory = OutputDirectory;
394+
target.FileName = FileName;
395+
target.DisableHMR = DisableHMR;
396+
target.HMRPort = HMRPort;
397+
target.ContainerImageTag = ContainerImageTag;
398+
target.CheckLatestImageVersion = CheckLatestImageVersion;
399+
target.AutoIconsEnabled = AutoIconsEnabled;
400+
target.HideFromDashboard = HideFromDashboard;
401+
target.DashboardLinkDisplayName = DashboardLinkDisplayName;
402+
target.RelationshipKindSyntax = RelationshipKindSyntax;
403+
target.FormatGeneratedFile = FormatGeneratedFile;
404+
target.ExternalProcessTimeoutSeconds = ExternalProcessTimeoutSeconds;
405+
target.UseDotIfAvailable = UseDotIfAvailable;
406+
target.AutoIncludeAspireMetadata = AutoIncludeAspireMetadata;
407+
target.NormaliseMetadataBehaviour = NormaliseMetadataBehaviour;
408+
target.GenerateConfigFile = GenerateConfigFile;
409+
target.IncludeAspireDashboardLinks = IncludeAspireDashboardLinks;
410+
target.IncludeAspireTokenInDashboardLinks = IncludeAspireTokenInDashboardLinks;
411+
target.IncludeDefaultStateStyles = IncludeDefaultStateStyles;
412+
413+
// Collection properties — create new instances to avoid shared mutable references.
414+
target.ElementKindSpecs = [.. ElementKindSpecs];
415+
target.RelationshipKindSpecs = [.. RelationshipKindSpecs];
416+
target.AdditionalDSLFiles = [.. AdditionalDSLFiles];
417+
target.AdditionalDSLFolders = [.. AdditionalDSLFolders];
418+
target.ExcludedResourceTypes = [.. ExcludedResourceTypes];
419+
420+
// IconResolvers has a getter-only List<T> — mutate in place, preserving order.
421+
target.IconResolvers.Clear();
422+
target.IconResolvers.AddRange(IconResolvers);
423+
424+
// Dictionary properties — new instances so callers can't mutate shared state.
425+
// Preserve each source dictionary's comparer so key-lookup semantics are unchanged
426+
// after the copy (e.g. ImageAliases uses OrdinalIgnoreCase by default).
427+
target.ImageAliases = new Dictionary<string, string>(ImageAliases, ImageAliases.Comparer);
428+
target.StateTagMap = new Dictionary<string, string?>(StateTagMap, StateTagMap.Comparer);
429+
target.ConfigFileMetadata = new Dictionary<string, string>(ConfigFileMetadata, ConfigFileMetadata.Comparer);
430+
}
431+
432+
/// <summary>
433+
/// Applies only the properties that differ from <paramref name="baseline"/> to <paramref name="target"/>.
434+
/// </summary>
435+
/// <remarks>
436+
/// Used by <c>AddAspireC4</c> to apply callback-authored overrides on top of a lazily-bound
437+
/// configuration pipeline. Because the callback is invoked eagerly against fresh defaults
438+
/// (both <c>this</c> and <paramref name="baseline"/> start from <c>new()</c>), any property
439+
/// that equals the baseline was not explicitly set by the callback and should not override a
440+
/// config-bound value. Only changed properties win, preserving correct config &lt; code precedence.
441+
/// </remarks>
442+
internal void ApplyDelta(AspireC4DiagramOptions baseline, AspireC4DiagramOptions target)
443+
{
444+
// Scalar properties — only apply if the callback changed them from the baseline default.
445+
if (GeneratedViewId != baseline.GeneratedViewId)
446+
target.GeneratedViewId = GeneratedViewId;
447+
if (DefaultViewId != baseline.DefaultViewId)
448+
target.DefaultViewId = DefaultViewId;
449+
if (Title != baseline.Title)
450+
target.Title = Title;
451+
if (ViewTitle != baseline.ViewTitle)
452+
target.ViewTitle = ViewTitle;
453+
if (ViewDescription != baseline.ViewDescription)
454+
target.ViewDescription = ViewDescription;
455+
if (OutputDirectory != baseline.OutputDirectory)
456+
target.OutputDirectory = OutputDirectory;
457+
if (FileName != baseline.FileName)
458+
target.FileName = FileName;
459+
if (DisableHMR != baseline.DisableHMR)
460+
target.DisableHMR = DisableHMR;
461+
if (HMRPort != baseline.HMRPort)
462+
target.HMRPort = HMRPort;
463+
if (ContainerImageTag != baseline.ContainerImageTag)
464+
target.ContainerImageTag = ContainerImageTag;
465+
if (CheckLatestImageVersion != baseline.CheckLatestImageVersion)
466+
target.CheckLatestImageVersion = CheckLatestImageVersion;
467+
if (AutoIconsEnabled != baseline.AutoIconsEnabled)
468+
target.AutoIconsEnabled = AutoIconsEnabled;
469+
if (HideFromDashboard != baseline.HideFromDashboard)
470+
target.HideFromDashboard = HideFromDashboard;
471+
if (DashboardLinkDisplayName != baseline.DashboardLinkDisplayName)
472+
target.DashboardLinkDisplayName = DashboardLinkDisplayName;
473+
if (RelationshipKindSyntax != baseline.RelationshipKindSyntax)
474+
target.RelationshipKindSyntax = RelationshipKindSyntax;
475+
if (FormatGeneratedFile != baseline.FormatGeneratedFile)
476+
target.FormatGeneratedFile = FormatGeneratedFile;
477+
if (ExternalProcessTimeoutSeconds != baseline.ExternalProcessTimeoutSeconds)
478+
target.ExternalProcessTimeoutSeconds = ExternalProcessTimeoutSeconds;
479+
if (UseDotIfAvailable != baseline.UseDotIfAvailable)
480+
target.UseDotIfAvailable = UseDotIfAvailable;
481+
if (AutoIncludeAspireMetadata != baseline.AutoIncludeAspireMetadata)
482+
target.AutoIncludeAspireMetadata = AutoIncludeAspireMetadata;
483+
if (NormaliseMetadataBehaviour != baseline.NormaliseMetadataBehaviour)
484+
target.NormaliseMetadataBehaviour = NormaliseMetadataBehaviour;
485+
if (GenerateConfigFile != baseline.GenerateConfigFile)
486+
target.GenerateConfigFile = GenerateConfigFile;
487+
if (IncludeAspireDashboardLinks != baseline.IncludeAspireDashboardLinks)
488+
target.IncludeAspireDashboardLinks = IncludeAspireDashboardLinks;
489+
if (IncludeAspireTokenInDashboardLinks != baseline.IncludeAspireTokenInDashboardLinks)
490+
target.IncludeAspireTokenInDashboardLinks = IncludeAspireTokenInDashboardLinks;
491+
if (IncludeDefaultStateStyles != baseline.IncludeDefaultStateStyles)
492+
target.IncludeDefaultStateStyles = IncludeDefaultStateStyles;
493+
494+
// Collection properties — apply if the count changed (any add/remove by the callback).
495+
if (ElementKindSpecs.Count != baseline.ElementKindSpecs.Count)
496+
target.ElementKindSpecs = [.. ElementKindSpecs];
497+
if (RelationshipKindSpecs.Count != baseline.RelationshipKindSpecs.Count)
498+
target.RelationshipKindSpecs = [.. RelationshipKindSpecs];
499+
if (AdditionalDSLFiles.Count != baseline.AdditionalDSLFiles.Count)
500+
target.AdditionalDSLFiles = [.. AdditionalDSLFiles];
501+
if (AdditionalDSLFolders.Count != baseline.AdditionalDSLFolders.Count)
502+
target.AdditionalDSLFolders = [.. AdditionalDSLFolders];
503+
504+
// Use SetEquals for ExcludedResourceTypes: its default is non-empty ({ParameterResource}),
505+
// so count comparison would yield false positives if the callback removes the default entry.
506+
if (!ExcludedResourceTypes.SetEquals(baseline.ExcludedResourceTypes))
507+
target.ExcludedResourceTypes = [.. ExcludedResourceTypes];
508+
509+
// IconResolvers has a getter-only List<T> — mutate in place.
510+
if (IconResolvers.Count != baseline.IconResolvers.Count)
511+
{
512+
target.IconResolvers.Clear();
513+
target.IconResolvers.AddRange(IconResolvers);
514+
}
515+
516+
// Dictionary properties — apply if count changed; preserve source comparer.
517+
if (ImageAliases.Count != baseline.ImageAliases.Count)
518+
target.ImageAliases = new Dictionary<string, string>(ImageAliases, ImageAliases.Comparer);
519+
if (StateTagMap.Count != baseline.StateTagMap.Count)
520+
target.StateTagMap = new Dictionary<string, string?>(StateTagMap, StateTagMap.Comparer);
521+
if (ConfigFileMetadata.Count != baseline.ConfigFileMetadata.Count)
522+
target.ConfigFileMetadata = new Dictionary<string, string>(ConfigFileMetadata, ConfigFileMetadata.Comparer);
523+
}
357524
}

‎src/src/AspireC4/Extensions/Aspire/Hosting/AspireC4DistributedApplicationBuilderExtensions.cs‎

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,30 +51,55 @@ public static IResourceBuilder<AspireC4Resource> AddAspireC4(
5151

5252
ArgumentNullException.ThrowIfNull(builder);
5353

54+
// Eagerly evaluate `configure` here — on the background thread that AddAspireC4 runs on
55+
// (this method is invoked via the ATS/TypeScript AppHost export path where
56+
// RunSyncOnBackgroundThread = true ensures a background thread; C# callers invoke this
57+
// directly and are not on the NonConcurrentSynchronizationContext). Doing so is safe because:
58+
// 1. The NonConcurrentSynchronizationContext is not occupied at this point.
59+
// 2. The ATS proxy for `configure` calls .GetAwaiter().GetResult() internally, but
60+
// the sync context is free so TypeScript's setter-call responses can be dispatched.
61+
//
62+
// We must NOT call configure?.Invoke(opts) inside the lazy IOptions.Configure callback
63+
// below: that callback may execute on the NonConcurrentSynchronizationContext (during
64+
// DistributedApplication.RunAsync), and .GetResult() would block it while waiting for
65+
// TypeScript's incoming setter calls — which themselves need the same blocked context.
66+
// Classic sync-over-async deadlock. See microsoft/aspire#17487.
67+
//
68+
// Strategy: capture a baseline (pure defaults) and invoke the callback against a second
69+
// fresh instance. In the lazy IOptions pipeline, BindConfiguration applies current config
70+
// (including any values added after this call returns), then ApplyDelta applies only the
71+
// properties the callback explicitly changed — preserving correct config < code precedence
72+
// without ever calling the ATS proxy on the sync context.
73+
var callbackBaseline = new AspireC4DiagramOptions();
74+
var callbackResult = new AspireC4DiagramOptions();
75+
configure?.Invoke(callbackResult);
76+
5477
builder
5578
.Services.AddOptions<AspireC4DiagramOptions>()
5679
.BindConfiguration(AspireC4DiagramOptions.SectionName)
5780
.Configure(opts =>
5881
{
59-
configure?.Invoke(opts);
82+
// Apply only the properties the callback changed relative to fresh defaults.
83+
// BindConfiguration (above) has already applied current configuration values;
84+
// ApplyDelta applies callback overrides on top, without invoking the ATS proxy.
85+
callbackResult.ApplyDelta(callbackBaseline, opts);
6086
opts.OutputDirectory = ResolveOutputDirectory(builder.AppHostDirectory, opts.OutputDirectory);
6187
});
6288

63-
AspireC4DiagramOptions diagramOpts = new();
64-
configure?.Invoke(diagramOpts);
65-
66-
var outputDir = ResolveOutputDirectory(builder.AppHostDirectory, diagramOpts.OutputDirectory);
89+
var outputDir = ResolveOutputDirectory(builder.AppHostDirectory, callbackResult.OutputDirectory);
6790
Directory.CreateDirectory(outputDir);
68-
var imageTag = diagramOpts.ContainerImageTag ?? LikeC4ServerResource.DefaultTag;
91+
var imageTag = callbackResult.ContainerImageTag ?? LikeC4ServerResource.DefaultTag;
6992
var hmrPortMode = HMRPortCompatibility.Resolve(imageTag);
70-
var resolvedHmrPort = diagramOpts.HMRPort ?? LikeC4ServerResource.DefaultContainerHMRPort;
71-
var defaultViewId = string.IsNullOrWhiteSpace(diagramOpts.DefaultViewId) ? null : diagramOpts.DefaultViewId;
93+
var resolvedHmrPort = callbackResult.HMRPort ?? LikeC4ServerResource.DefaultContainerHMRPort;
94+
var defaultViewId = string.IsNullOrWhiteSpace(callbackResult.DefaultViewId)
95+
? null
96+
: callbackResult.DefaultViewId;
7297

7398
// Only create a version probe when using "latest" with version checking enabled.
7499
// A pinned tag always has a known HMR mode; "latest" requires a probe to discover it.
75100
var needsVersionProbe =
76101
string.Equals(imageTag, LikeC4ServerResource.DefaultTag, StringComparison.OrdinalIgnoreCase)
77-
&& diagramOpts.CheckLatestImageVersion;
102+
&& callbackResult.CheckLatestImageVersion;
78103

79104
// Pre-complete the TCS when no probe is needed so WithArgs can proceed without waiting.
80105
var hmrPortModeTcs = new TaskCompletionSource<HMRPortMode>(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -91,7 +116,7 @@ public static IResourceBuilder<AspireC4Resource> AddAspireC4(
91116
// Dynamic (null) host ports cannot work here: if Docker maps host:DYNAMIC → container:24678
92117
// but Vite is told --hmr-port DYNAMIC it binds to container:DYNAMIC, which Docker doesn't
93118
// forward, breaking the HMR WebSocket connection entirely.
94-
int? hmrHostPort = diagramOpts.HMRPort ?? resolvedHmrPort;
119+
int? hmrHostPort = callbackResult.HMRPort ?? resolvedHmrPort;
95120

96121
builder
97122
.Services.AddOptions<ContainerWorkspaceOptions>()
@@ -156,10 +181,10 @@ public static IResourceBuilder<AspireC4Resource> AddAspireC4(
156181
context.Args.Add("start");
157182
context.Args.Add(wsOpts.Value.ContainerServePath);
158183

159-
if (!string.IsNullOrWhiteSpace(diagramOpts.Title))
184+
if (!string.IsNullOrWhiteSpace(callbackResult.Title))
160185
{
161186
context.Args.Add("--title");
162-
context.Args.Add($"\"{diagramOpts.Title}\"");
187+
context.Args.Add($"\"{callbackResult.Title}\"");
163188
}
164189

165190
var useDot =
@@ -190,7 +215,7 @@ public static IResourceBuilder<AspireC4Resource> AddAspireC4(
190215
.WithAnnotation(new LikeC4DslIdAnnotation(name))
191216
.ExcludeFromManifest();
192217

193-
if (!diagramOpts.DisableHMR)
218+
if (!callbackResult.DisableHMR)
194219
{
195220
serverBuilder
196221
.WithHttpEndpoint(

‎src/src/AspireC4/Lifecycle/AspireC4LifecycleHook.ContainerBindMount.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,19 @@ sealed partial class AspireC4LifecycleHook
1010
/// </summary>
1111
void SetupContainerBindMount(DistributedApplicationModel _, LikeC4ServerResource serverResource)
1212
{
13+
// Log before resolving options so that a future deadlock in this area (e.g. if the
14+
// lazy IOptions.Configure callback ever blocks on the NonConcurrentSynchronizationContext)
15+
// leaves a clear breadcrumb in the log rather than silent 60-second timeout.
16+
// See: https://github.com/microsoft/aspire/issues/17487
17+
telemetry.ApplyingDiagramOptionsSnapshot();
18+
1319
var opts = options.Value;
20+
21+
telemetry.DiagramOptionsSnapshotApplied(
22+
Path.GetFileName(opts.OutputDirectory),
23+
opts.FormatGeneratedFile,
24+
opts.DisableHMR
25+
);
1426
var outputDir = Path.GetFullPath(opts.OutputDirectory);
1527

1628
// Collect all host-side directory paths that must be visible inside the container.

‎src/src/AspireC4/Lifecycle/IAspireC4LifecycleHookTelemetry.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,10 @@ interface IAspireC4LifecycleHookTelemetry
4040

4141
[Warning]
4242
void FailedToResolveLatestContainerVersion();
43+
44+
[Debug]
45+
void ApplyingDiagramOptionsSnapshot();
46+
47+
[Debug]
48+
void DiagramOptionsSnapshotApplied(string outputDirectoryName, bool formatGeneratedFile, bool disableHmr);
4349
}

0 commit comments

Comments
 (0)