Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 167 additions & 0 deletions src/src/AspireC4/AspireC4DiagramOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -354,4 +354,171 @@ public sealed class AspireC4DiagramOptions
/// </summary>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2227:Collection properties should be read only")]
public Dictionary<string, string> ConfigFileMetadata { get; set; } = [];

/// <summary>
/// Copies all property values from this instance to <paramref name="target"/>.
/// </summary>
/// <remarks>
/// <para>
/// This method is called inside the lazy <c>IOptions.Configure</c> callback registered by
/// <c>AddAspireC4</c>, where we must NOT invoke the user's <c>configure</c> delegate directly.
/// </para>
/// <para>
/// <b>Why we can't call <c>configure?.Invoke(opts)</c> lazily:</b> in the polyglot (TypeScript)
/// AppHost scenario, <c>configure</c> is an ATS-generated proxy for an <c>async</c> TypeScript
/// callback (e.g. <c>async (opts) =&gt; { await opts.title.set("…"); }</c>). That proxy calls
/// <c>InvokeAsync(…).GetAwaiter().GetResult()</c> internally to bridge the async boundary.
/// When <c>IOptions.Value</c> is accessed during <c>DistributedApplication.RunAsync</c>, the
/// <c>Configure</c> callback executes on StreamJsonRpc's
/// <c>NonConcurrentSynchronizationContext</c> (a single-item dispatch queue). The
/// <c>.GetResult()</c> call blocks that context while waiting for TypeScript's incoming
/// setter calls — which themselves need to be dispatched on the same blocked context.
/// Classic sync-over-async deadlock.
/// </para>
/// <para>
/// The fix: <c>AddAspireC4</c> eagerly invokes <c>configure</c> on a background thread
/// (safe because <c>RunSyncOnBackgroundThread = true</c>), captures the fully-materialised
/// options snapshot (defaults → config binding → user callback), and then uses
/// <c>CopyTo</c> here to apply that snapshot in the lazy callback — no ATS proxy involved.
/// </para>
/// </remarks>
internal void CopyTo(AspireC4DiagramOptions target)
{
// Scalar / simple-value properties
target.GeneratedViewId = GeneratedViewId;
target.DefaultViewId = DefaultViewId;
target.Title = Title;
target.ViewTitle = ViewTitle;
target.ViewDescription = ViewDescription;
target.OutputDirectory = OutputDirectory;
target.FileName = FileName;
target.DisableHMR = DisableHMR;
target.HMRPort = HMRPort;
target.ContainerImageTag = ContainerImageTag;
target.CheckLatestImageVersion = CheckLatestImageVersion;
target.AutoIconsEnabled = AutoIconsEnabled;
target.HideFromDashboard = HideFromDashboard;
target.DashboardLinkDisplayName = DashboardLinkDisplayName;
target.RelationshipKindSyntax = RelationshipKindSyntax;
target.FormatGeneratedFile = FormatGeneratedFile;
target.ExternalProcessTimeoutSeconds = ExternalProcessTimeoutSeconds;
target.UseDotIfAvailable = UseDotIfAvailable;
target.AutoIncludeAspireMetadata = AutoIncludeAspireMetadata;
target.NormaliseMetadataBehaviour = NormaliseMetadataBehaviour;
target.GenerateConfigFile = GenerateConfigFile;
target.IncludeAspireDashboardLinks = IncludeAspireDashboardLinks;
target.IncludeAspireTokenInDashboardLinks = IncludeAspireTokenInDashboardLinks;
target.IncludeDefaultStateStyles = IncludeDefaultStateStyles;

// Collection properties — create new instances to avoid shared mutable references.
target.ElementKindSpecs = [.. ElementKindSpecs];
target.RelationshipKindSpecs = [.. RelationshipKindSpecs];
target.AdditionalDSLFiles = [.. AdditionalDSLFiles];
target.AdditionalDSLFolders = [.. AdditionalDSLFolders];
target.ExcludedResourceTypes = [.. ExcludedResourceTypes];

// IconResolvers has a getter-only List<T> — mutate in place, preserving order.
target.IconResolvers.Clear();
target.IconResolvers.AddRange(IconResolvers);

// Dictionary properties — new instances so callers can't mutate shared state.
// Preserve each source dictionary's comparer so key-lookup semantics are unchanged
// after the copy (e.g. ImageAliases uses OrdinalIgnoreCase by default).
target.ImageAliases = new Dictionary<string, string>(ImageAliases, ImageAliases.Comparer);
target.StateTagMap = new Dictionary<string, string?>(StateTagMap, StateTagMap.Comparer);
target.ConfigFileMetadata = new Dictionary<string, string>(ConfigFileMetadata, ConfigFileMetadata.Comparer);
}

/// <summary>
/// Applies only the properties that differ from <paramref name="baseline"/> to <paramref name="target"/>.
/// </summary>
/// <remarks>
/// Used by <c>AddAspireC4</c> to apply callback-authored overrides on top of a lazily-bound
/// configuration pipeline. Because the callback is invoked eagerly against fresh defaults
/// (both <c>this</c> and <paramref name="baseline"/> start from <c>new()</c>), any property
/// that equals the baseline was not explicitly set by the callback and should not override a
/// config-bound value. Only changed properties win, preserving correct config &lt; code precedence.
/// </remarks>
internal void ApplyDelta(AspireC4DiagramOptions baseline, AspireC4DiagramOptions target)
{
// Scalar properties — only apply if the callback changed them from the baseline default.
if (GeneratedViewId != baseline.GeneratedViewId)
target.GeneratedViewId = GeneratedViewId;
if (DefaultViewId != baseline.DefaultViewId)
target.DefaultViewId = DefaultViewId;
if (Title != baseline.Title)
target.Title = Title;
if (ViewTitle != baseline.ViewTitle)
target.ViewTitle = ViewTitle;
if (ViewDescription != baseline.ViewDescription)
target.ViewDescription = ViewDescription;
if (OutputDirectory != baseline.OutputDirectory)
target.OutputDirectory = OutputDirectory;
if (FileName != baseline.FileName)
target.FileName = FileName;
if (DisableHMR != baseline.DisableHMR)
target.DisableHMR = DisableHMR;
if (HMRPort != baseline.HMRPort)
target.HMRPort = HMRPort;
if (ContainerImageTag != baseline.ContainerImageTag)
target.ContainerImageTag = ContainerImageTag;
if (CheckLatestImageVersion != baseline.CheckLatestImageVersion)
target.CheckLatestImageVersion = CheckLatestImageVersion;
if (AutoIconsEnabled != baseline.AutoIconsEnabled)
target.AutoIconsEnabled = AutoIconsEnabled;
if (HideFromDashboard != baseline.HideFromDashboard)
target.HideFromDashboard = HideFromDashboard;
if (DashboardLinkDisplayName != baseline.DashboardLinkDisplayName)
target.DashboardLinkDisplayName = DashboardLinkDisplayName;
if (RelationshipKindSyntax != baseline.RelationshipKindSyntax)
target.RelationshipKindSyntax = RelationshipKindSyntax;
if (FormatGeneratedFile != baseline.FormatGeneratedFile)
target.FormatGeneratedFile = FormatGeneratedFile;
if (ExternalProcessTimeoutSeconds != baseline.ExternalProcessTimeoutSeconds)
target.ExternalProcessTimeoutSeconds = ExternalProcessTimeoutSeconds;
if (UseDotIfAvailable != baseline.UseDotIfAvailable)
target.UseDotIfAvailable = UseDotIfAvailable;
if (AutoIncludeAspireMetadata != baseline.AutoIncludeAspireMetadata)
target.AutoIncludeAspireMetadata = AutoIncludeAspireMetadata;
if (NormaliseMetadataBehaviour != baseline.NormaliseMetadataBehaviour)
target.NormaliseMetadataBehaviour = NormaliseMetadataBehaviour;
if (GenerateConfigFile != baseline.GenerateConfigFile)
target.GenerateConfigFile = GenerateConfigFile;
if (IncludeAspireDashboardLinks != baseline.IncludeAspireDashboardLinks)
target.IncludeAspireDashboardLinks = IncludeAspireDashboardLinks;
if (IncludeAspireTokenInDashboardLinks != baseline.IncludeAspireTokenInDashboardLinks)
target.IncludeAspireTokenInDashboardLinks = IncludeAspireTokenInDashboardLinks;
if (IncludeDefaultStateStyles != baseline.IncludeDefaultStateStyles)
target.IncludeDefaultStateStyles = IncludeDefaultStateStyles;

// Collection properties — apply if the count changed (any add/remove by the callback).
if (ElementKindSpecs.Count != baseline.ElementKindSpecs.Count)
target.ElementKindSpecs = [.. ElementKindSpecs];
if (RelationshipKindSpecs.Count != baseline.RelationshipKindSpecs.Count)
target.RelationshipKindSpecs = [.. RelationshipKindSpecs];
if (AdditionalDSLFiles.Count != baseline.AdditionalDSLFiles.Count)
target.AdditionalDSLFiles = [.. AdditionalDSLFiles];
if (AdditionalDSLFolders.Count != baseline.AdditionalDSLFolders.Count)
target.AdditionalDSLFolders = [.. AdditionalDSLFolders];

// Use SetEquals for ExcludedResourceTypes: its default is non-empty ({ParameterResource}),
// so count comparison would yield false positives if the callback removes the default entry.
if (!ExcludedResourceTypes.SetEquals(baseline.ExcludedResourceTypes))
target.ExcludedResourceTypes = [.. ExcludedResourceTypes];

// IconResolvers has a getter-only List<T> — mutate in place.
if (IconResolvers.Count != baseline.IconResolvers.Count)
{
target.IconResolvers.Clear();
target.IconResolvers.AddRange(IconResolvers);
}

// Dictionary properties — apply if count changed; preserve source comparer.
if (ImageAliases.Count != baseline.ImageAliases.Count)
target.ImageAliases = new Dictionary<string, string>(ImageAliases, ImageAliases.Comparer);
if (StateTagMap.Count != baseline.StateTagMap.Count)
target.StateTagMap = new Dictionary<string, string?>(StateTagMap, StateTagMap.Comparer);
if (ConfigFileMetadata.Count != baseline.ConfigFileMetadata.Count)
target.ConfigFileMetadata = new Dictionary<string, string>(ConfigFileMetadata, ConfigFileMetadata.Comparer);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,30 +51,55 @@ public static IResourceBuilder<AspireC4Resource> AddAspireC4(

ArgumentNullException.ThrowIfNull(builder);

// Eagerly evaluate `configure` here — on the background thread that AddAspireC4 runs on
// (this method is invoked via the ATS/TypeScript AppHost export path where
// RunSyncOnBackgroundThread = true ensures a background thread; C# callers invoke this
// directly and are not on the NonConcurrentSynchronizationContext). Doing so is safe because:
// 1. The NonConcurrentSynchronizationContext is not occupied at this point.
// 2. The ATS proxy for `configure` calls .GetAwaiter().GetResult() internally, but
// the sync context is free so TypeScript's setter-call responses can be dispatched.
//
// We must NOT call configure?.Invoke(opts) inside the lazy IOptions.Configure callback
// below: that callback may execute on the NonConcurrentSynchronizationContext (during
// DistributedApplication.RunAsync), and .GetResult() would block it while waiting for
// TypeScript's incoming setter calls — which themselves need the same blocked context.
// Classic sync-over-async deadlock. See microsoft/aspire#17487.
//
// Strategy: capture a baseline (pure defaults) and invoke the callback against a second
// fresh instance. In the lazy IOptions pipeline, BindConfiguration applies current config
// (including any values added after this call returns), then ApplyDelta applies only the
// properties the callback explicitly changed — preserving correct config < code precedence
// without ever calling the ATS proxy on the sync context.
var callbackBaseline = new AspireC4DiagramOptions();
var callbackResult = new AspireC4DiagramOptions();
configure?.Invoke(callbackResult);

builder
.Services.AddOptions<AspireC4DiagramOptions>()
.BindConfiguration(AspireC4DiagramOptions.SectionName)
.Configure(opts =>
{
configure?.Invoke(opts);
// Apply only the properties the callback changed relative to fresh defaults.
// BindConfiguration (above) has already applied current configuration values;
// ApplyDelta applies callback overrides on top, without invoking the ATS proxy.
callbackResult.ApplyDelta(callbackBaseline, opts);
opts.OutputDirectory = ResolveOutputDirectory(builder.AppHostDirectory, opts.OutputDirectory);
});

AspireC4DiagramOptions diagramOpts = new();
configure?.Invoke(diagramOpts);

var outputDir = ResolveOutputDirectory(builder.AppHostDirectory, diagramOpts.OutputDirectory);
var outputDir = ResolveOutputDirectory(builder.AppHostDirectory, callbackResult.OutputDirectory);
Directory.CreateDirectory(outputDir);
var imageTag = diagramOpts.ContainerImageTag ?? LikeC4ServerResource.DefaultTag;
var imageTag = callbackResult.ContainerImageTag ?? LikeC4ServerResource.DefaultTag;
var hmrPortMode = HMRPortCompatibility.Resolve(imageTag);
var resolvedHmrPort = diagramOpts.HMRPort ?? LikeC4ServerResource.DefaultContainerHMRPort;
var defaultViewId = string.IsNullOrWhiteSpace(diagramOpts.DefaultViewId) ? null : diagramOpts.DefaultViewId;
var resolvedHmrPort = callbackResult.HMRPort ?? LikeC4ServerResource.DefaultContainerHMRPort;
var defaultViewId = string.IsNullOrWhiteSpace(callbackResult.DefaultViewId)
? null
: callbackResult.DefaultViewId;

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

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

builder
.Services.AddOptions<ContainerWorkspaceOptions>()
Expand Down Expand Up @@ -156,10 +181,10 @@ public static IResourceBuilder<AspireC4Resource> AddAspireC4(
context.Args.Add("start");
context.Args.Add(wsOpts.Value.ContainerServePath);

if (!string.IsNullOrWhiteSpace(diagramOpts.Title))
if (!string.IsNullOrWhiteSpace(callbackResult.Title))
{
context.Args.Add("--title");
context.Args.Add($"\"{diagramOpts.Title}\"");
context.Args.Add($"\"{callbackResult.Title}\"");
}

var useDot =
Expand Down Expand Up @@ -190,7 +215,7 @@ public static IResourceBuilder<AspireC4Resource> AddAspireC4(
.WithAnnotation(new LikeC4DslIdAnnotation(name))
.ExcludeFromManifest();

if (!diagramOpts.DisableHMR)
if (!callbackResult.DisableHMR)
{
serverBuilder
.WithHttpEndpoint(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,19 @@ sealed partial class AspireC4LifecycleHook
/// </summary>
void SetupContainerBindMount(DistributedApplicationModel _, LikeC4ServerResource serverResource)
{
// Log before resolving options so that a future deadlock in this area (e.g. if the
// lazy IOptions.Configure callback ever blocks on the NonConcurrentSynchronizationContext)
// leaves a clear breadcrumb in the log rather than silent 60-second timeout.
// See: https://github.com/microsoft/aspire/issues/17487
telemetry.ApplyingDiagramOptionsSnapshot();

var opts = options.Value;

telemetry.DiagramOptionsSnapshotApplied(
Path.GetFileName(opts.OutputDirectory),
opts.FormatGeneratedFile,
opts.DisableHMR
);
var outputDir = Path.GetFullPath(opts.OutputDirectory);

// Collect all host-side directory paths that must be visible inside the container.
Expand Down
6 changes: 6 additions & 0 deletions src/src/AspireC4/Lifecycle/IAspireC4LifecycleHookTelemetry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,10 @@ interface IAspireC4LifecycleHookTelemetry

[Warning]
void FailedToResolveLatestContainerVersion();

[Debug]
void ApplyingDiagramOptionsSnapshot();

[Debug]
void DiagramOptionsSnapshotApplied(string outputDirectoryName, bool formatGeneratedFile, bool disableHmr);
}
Loading