Skip to content

Commit 422f881

Browse files
davidwengierdavidwengier
authored
Retry running generators if the project config looks good, but the source generator doesn't produce host outputs (#12228)
Fixes the startup issue we have if Razor loses the race condition, and Roslyn caches the generator run result while it's in "cohosting off" mode. Essentially our snapshot manager, which is our first port of call for "I would like to run this OOP service for this solution snapshot" will not sometimes redirect calls to a forked snapshot, that ensures the generator will have had a chance to re-run after it has seen the cohosting flag turn on. Some minor changes to solution and project snapshots because this could happen while code is in-flight, but after the first requests are done, subsequent requests flow through the system as normal (albeit being silently redirected to the fork) and once almost any change to the project or additional files has happened, the whole "retry project" notion becomes unnecessary. Val build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2793362&view=results Test insertion: https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/670224
2 parents f5cf25b + 1cde74f commit 422f881

7 files changed

Lines changed: 364 additions & 82 deletions

File tree

‎src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/Extensions.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
5+
using System.Collections.Generic;
56
using System.Linq;
67
using Microsoft.AspNetCore.Razor;
78
using Microsoft.AspNetCore.Razor.Language;
@@ -12,6 +13,8 @@ namespace Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem;
1213

1314
internal static class Extensions
1415
{
16+
private const string RetryProjectFeatureName = "__razor_cohost_retry";
17+
1518
public static bool IsRazorDocument(this TextDocument document)
1619
=> document is AdditionalDocument &&
1720
document.FilePath is string filePath &&
@@ -27,4 +30,11 @@ public static Uri GetRazorDocumentUri(this Solution solution, RazorCodeDocument
2730
var document = solution.GetAdditionalDocument(documentId).AssumeNotNull();
2831
return document.CreateUri();
2932
}
33+
34+
public static Project ForkToRetryProject(this Project project)
35+
// Using a feature to act as a no-op change so we can't accidentally break something
36+
=> project.WithParseOptions(project.ParseOptions.AssumeNotNull().WithFeatures([.. project.ParseOptions.Features, new KeyValuePair<string, string>(RetryProjectFeatureName, "")]));
37+
38+
public static bool IsRetryProject(this Project project)
39+
=> project.ParseOptions.AssumeNotNull().Features.ContainsKey(RetryProjectFeatureName);
3040
}

‎src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/GeneratorRunResult.cs‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Collections.Immutable;
7+
using System.Linq;
8+
using System.Threading;
9+
using System.Threading.Tasks;
10+
using Microsoft.AspNetCore.Razor.Language;
11+
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
12+
using Microsoft.NET.Sdk.Razor.SourceGenerators;
13+
14+
namespace Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem;
15+
16+
/// <summary>
17+
/// Wraps the host outputs of the Razor source generator, and the <see cref="CodeAnalysis.Project"/> that produced them.
18+
/// </summary>
19+
internal readonly record struct GeneratorRunResult(RazorGeneratorResult GeneratorResult, Project Project)
20+
{
21+
public bool IsDefault => GeneratorResult is null && Project is null;
22+
23+
public IReadOnlyList<TagHelperDescriptor> TagHelpers => GeneratorResult.TagHelpers;
24+
25+
public RazorCodeDocument? GetCodeDocument(string filePath)
26+
=> GeneratorResult.GetCodeDocument(filePath);
27+
28+
public RazorCodeDocument GetRequiredCodeDocument(string filePath)
29+
=> GeneratorResult.GetCodeDocument(filePath)
30+
?? throw new InvalidOperationException(SR.FormatGenerator_run_result_did_not_contain_a_code_document(filePath));
31+
32+
public string? GetRazorFilePathFromHintName(string generatedDocumentHintName)
33+
=> GeneratorResult.GetFilePath(generatedDocumentHintName);
34+
35+
public bool TryGetRazorDocument(string razorFilePath, out TextDocument? document)
36+
=> Project.Solution.TryGetRazorDocument(razorFilePath, out document);
37+
38+
public async Task<SourceGeneratedDocument> GetRequiredSourceGeneratedDocumentForRazorFilePathAsync(string filePath, CancellationToken cancellationToken)
39+
{
40+
var hintName = GeneratorResult.GetHintName(filePath);
41+
42+
var generatedDocument = await Project.TryGetSourceGeneratedDocumentFromHintNameAsync(hintName, cancellationToken).ConfigureAwait(false);
43+
44+
return generatedDocument
45+
?? throw new InvalidOperationException(SR.FormatCouldnt_get_the_source_generated_document_for_hint_name(hintName));
46+
}
47+
48+
public static async Task<GeneratorRunResult> CreateAsync(bool throwIfNotFound, Project project, RemoteSnapshotManager snapshotManager, CancellationToken cancellationToken)
49+
{
50+
var result = await project.GetSourceGeneratorRunResultAsync(cancellationToken).ConfigureAwait(false);
51+
if (result is null)
52+
{
53+
if (throwIfNotFound)
54+
{
55+
throw new InvalidOperationException(SR.FormatCouldnt_get_a_source_generator_run_result(project.Name));
56+
}
57+
58+
return default;
59+
}
60+
61+
var runResult = result.Results.SingleOrDefault(r => r.Generator.GetGeneratorType().Assembly.Location == typeof(RazorSourceGenerator).Assembly.Location);
62+
if (runResult.Generator is null)
63+
{
64+
if (throwIfNotFound)
65+
{
66+
if (result.Results.SingleOrDefault(r => r.Generator.GetGeneratorType().Name == "Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator").Generator is { } wrongGenerator)
67+
{
68+
// Wrong ALC?
69+
throw new InvalidOperationException(SR.FormatRazor_source_generator_reference_incorrect(wrongGenerator.GetGeneratorType().Assembly.Location, typeof(RazorSourceGenerator).Assembly.Location, project.Name));
70+
}
71+
else
72+
{
73+
throw new InvalidOperationException(SR.FormatRazor_source_generator_is_not_referenced(project.Name));
74+
}
75+
}
76+
77+
return default;
78+
}
79+
80+
#pragma warning disable RSEXPERIMENTAL004 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
81+
if (!runResult.HostOutputs.TryGetValue(nameof(RazorGeneratorResult), out var objectResult))
82+
#pragma warning restore RSEXPERIMENTAL004 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
83+
{
84+
// We know the generator is referenced, or we wouldn't have gotten past the above checks. We also know cohosting is turned on, since we got here.
85+
// There is a race condition that can happen where if Roslyn runs the generator before Razor has a chance to initialize, then Roslyn will have
86+
// cached the fact that cohosting was off, and any subsequent runs of the generator will not produce a host output. We can work around this by
87+
// making an innocuous change to the project, and trying again. The snapshot manager makes this change inside a lock, and if we're not the first
88+
// ones to try, we'll get the updated project which may have already had generators run on it. We recurse back into ourselves here with the updated
89+
// project to try again. TryGetRetryProject also protects against infinite recursion.
90+
if (snapshotManager.TryGetRetryProject(project) is { } retryProject)
91+
{
92+
return await CreateAsync(throwIfNotFound, retryProject, snapshotManager, cancellationToken).ConfigureAwait(false);
93+
}
94+
95+
if (throwIfNotFound)
96+
{
97+
throw new InvalidOperationException(SR.FormatRazor_source_generator_did_not_produce_a_host_output(project.Name, string.Join(Environment.NewLine, runResult.Diagnostics)));
98+
}
99+
100+
return default;
101+
}
102+
103+
if (objectResult is not RazorGeneratorResult generatorResult)
104+
{
105+
if (throwIfNotFound)
106+
{
107+
// Wrong ALC?
108+
throw new InvalidOperationException(SR.FormatRazor_source_generator_host_output_is_not_RazorGeneratorResult(project.Name, string.Join(Environment.NewLine, runResult.Diagnostics)));
109+
}
110+
111+
return default;
112+
}
113+
114+
return new(generatorResult, project);
115+
}
116+
}

‎src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteProjectSnapshot.cs‎

Lines changed: 24 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,9 @@
1212
using Microsoft.AspNetCore.Razor.Language;
1313
using Microsoft.CodeAnalysis;
1414
using Microsoft.CodeAnalysis.CSharp;
15-
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
1615
using Microsoft.CodeAnalysis.Razor;
1716
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
1817
using Microsoft.CodeAnalysis.Razor.Utilities;
19-
using Microsoft.NET.Sdk.Razor.SourceGenerators;
2018

2119
namespace Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem;
2220

@@ -60,8 +58,8 @@ public IEnumerable<string> DocumentFilePaths
6058

6159
public async ValueTask<ImmutableArray<TagHelperDescriptor>> GetTagHelpersAsync(CancellationToken cancellationToken)
6260
{
63-
var generatorResult = await GetRazorGeneratorResultAsync(throwIfNotFound: false, cancellationToken).ConfigureAwait(false);
64-
if (generatorResult is null)
61+
var generatorResult = await GeneratorRunResult.CreateAsync(throwIfNotFound: false, _project, SolutionSnapshot.SnapshotManager, cancellationToken).ConfigureAwait(false);
62+
if (generatorResult.IsDefault)
6563
return [];
6664

6765
return [.. generatorResult.TagHelpers];
@@ -71,7 +69,13 @@ public RemoteDocumentSnapshot GetDocument(TextDocument document)
7169
{
7270
if (document.Project != _project)
7371
{
74-
throw new ArgumentException(SR.Document_does_not_belong_to_this_project, nameof(document));
72+
// We got asked for a document that doesn't belong to this project, but it could be that we are the result
73+
// of re-running the generator (a "retry project") and the document they passed in is from the original project
74+
// because it comes from the devenv side, so we can be a little lenient here. Since retry projects only exist
75+
// early in a session, we should still catch coding errors with even basic manual testing.
76+
document = _project.IsRetryProject() && _project.GetAdditionalDocument(document.Id) is { } retryDocument
77+
? retryDocument
78+
: throw new ArgumentException(SR.Document_does_not_belong_to_this_project, nameof(document));
7579
}
7680

7781
if (!document.IsRazorDocument())
@@ -142,22 +146,20 @@ public bool TryGetDocument(string filePath, [NotNullWhen(true)] out IDocumentSna
142146

143147
internal async Task<RazorCodeDocument> GetRequiredCodeDocumentAsync(IDocumentSnapshot documentSnapshot, CancellationToken cancellationToken)
144148
{
145-
var generatorResult = await GetRazorGeneratorResultAsync(throwIfNotFound: true, cancellationToken).ConfigureAwait(false);
149+
var generatorResult = await GeneratorRunResult.CreateAsync(throwIfNotFound: true, _project, SolutionSnapshot.SnapshotManager, cancellationToken).ConfigureAwait(false);
146150

147-
return generatorResult.AssumeNotNull().GetCodeDocument(documentSnapshot.FilePath)
148-
?? throw new InvalidOperationException(SR.FormatGenerator_run_result_did_not_contain_a_code_document(documentSnapshot.FilePath));
151+
Assumed.False(generatorResult.IsDefault);
152+
153+
return generatorResult.GetRequiredCodeDocument(documentSnapshot.FilePath);
149154
}
150155

151156
internal async Task<SourceGeneratedDocument> GetRequiredGeneratedDocumentAsync(IDocumentSnapshot documentSnapshot, CancellationToken cancellationToken)
152157
{
153-
var generatorResult = await GetRazorGeneratorResultAsync(throwIfNotFound: true, cancellationToken).ConfigureAwait(false);
154-
155-
var hintName = generatorResult.AssumeNotNull().GetHintName(documentSnapshot.FilePath);
158+
var generatorResult = await GeneratorRunResult.CreateAsync(throwIfNotFound: true, _project, SolutionSnapshot.SnapshotManager, cancellationToken).ConfigureAwait(false);
156159

157-
var generatedDocument = await _project.TryGetSourceGeneratedDocumentFromHintNameAsync(hintName, cancellationToken).ConfigureAwait(false);
160+
Assumed.False(generatorResult.IsDefault);
158161

159-
return generatedDocument
160-
?? throw new InvalidOperationException(SR.FormatCouldnt_get_the_source_generated_document_for_hint_name(hintName));
162+
return await generatorResult.GetRequiredSourceGeneratedDocumentForRazorFilePathAsync(documentSnapshot.FilePath, cancellationToken).ConfigureAwait(false);
161163
}
162164

163165
public async Task<RazorCodeDocument?> TryGetCodeDocumentFromGeneratedDocumentUriAsync(Uri generatedDocumentUri, CancellationToken cancellationToken)
@@ -172,86 +174,28 @@ internal async Task<SourceGeneratedDocument> GetRequiredGeneratedDocumentAsync(I
172174

173175
public async Task<RazorCodeDocument?> TryGetCodeDocumentFromGeneratedHintNameAsync(string generatedDocumentHintName, CancellationToken cancellationToken)
174176
{
175-
var runResult = await GetRazorGeneratorResultAsync(throwIfNotFound: false, cancellationToken).ConfigureAwait(false);
176-
if (runResult is null)
177+
var generatorResult = await GeneratorRunResult.CreateAsync(throwIfNotFound: false, _project, SolutionSnapshot.SnapshotManager, cancellationToken).ConfigureAwait(false);
178+
if (generatorResult.IsDefault)
177179
{
178180
return null;
179181
}
180182

181-
return runResult.GetFilePath(generatedDocumentHintName) is { } razorFilePath
182-
? runResult.GetCodeDocument(razorFilePath)
183+
return generatorResult.GetRazorFilePathFromHintName(generatedDocumentHintName) is { } razorFilePath
184+
? generatorResult.GetCodeDocument(razorFilePath)
183185
: null;
184186
}
185187

186188
public async Task<TextDocument?> TryGetRazorDocumentFromGeneratedHintNameAsync(string generatedDocumentHintName, CancellationToken cancellationToken)
187189
{
188-
var runResult = await GetRazorGeneratorResultAsync(throwIfNotFound: false, cancellationToken).ConfigureAwait(false);
189-
if (runResult is null)
190+
var generatorResult = await GeneratorRunResult.CreateAsync(throwIfNotFound: false, _project, SolutionSnapshot.SnapshotManager, cancellationToken).ConfigureAwait(false);
191+
if (generatorResult.IsDefault)
190192
{
191193
return null;
192194
}
193195

194-
return runResult.GetFilePath(generatedDocumentHintName) is { } razorFilePath &&
195-
_project.Solution.TryGetRazorDocument(razorFilePath, out var razorDocument)
196+
return generatorResult.GetRazorFilePathFromHintName(generatedDocumentHintName) is { } razorFilePath &&
197+
generatorResult.TryGetRazorDocument(razorFilePath, out var razorDocument)
196198
? razorDocument
197199
: null;
198200
}
199-
200-
private async Task<RazorGeneratorResult?> GetRazorGeneratorResultAsync(bool throwIfNotFound, CancellationToken cancellationToken)
201-
{
202-
var result = await _project.GetSourceGeneratorRunResultAsync(cancellationToken).ConfigureAwait(false);
203-
if (result is null)
204-
{
205-
if (throwIfNotFound)
206-
{
207-
throw new InvalidOperationException(SR.FormatCouldnt_get_a_source_generator_run_result(_project.Name));
208-
}
209-
210-
return null;
211-
}
212-
213-
var runResult = result.Results.SingleOrDefault(r => r.Generator.GetGeneratorType().Assembly.Location == typeof(RazorSourceGenerator).Assembly.Location);
214-
if (runResult.Generator is null)
215-
{
216-
if (throwIfNotFound)
217-
{
218-
if (result.Results.SingleOrDefault(r => r.Generator.GetGeneratorType().Name == "Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator").Generator is { } wrongGenerator)
219-
{
220-
// Wrong ALC?
221-
throw new InvalidOperationException(SR.FormatRazor_source_generator_reference_incorrect(wrongGenerator.GetGeneratorType().Assembly.Location, typeof(RazorSourceGenerator).Assembly.Location, _project.Name));
222-
}
223-
else
224-
{
225-
throw new InvalidOperationException(SR.FormatRazor_source_generator_is_not_referenced(_project.Name));
226-
}
227-
}
228-
229-
return null;
230-
}
231-
232-
#pragma warning disable RSEXPERIMENTAL004 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
233-
if (!runResult.HostOutputs.TryGetValue(nameof(RazorGeneratorResult), out var objectResult))
234-
#pragma warning restore RSEXPERIMENTAL004 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
235-
{
236-
if (throwIfNotFound)
237-
{
238-
throw new InvalidOperationException(SR.FormatRazor_source_generator_did_not_produce_a_host_output(_project.Name, string.Join(Environment.NewLine, runResult.Diagnostics)));
239-
}
240-
241-
return null;
242-
}
243-
244-
if (objectResult is not RazorGeneratorResult generatorResult)
245-
{
246-
if (throwIfNotFound)
247-
{
248-
// Wrong ALC?
249-
throw new InvalidOperationException(SR.FormatRazor_source_generator_host_output_is_not_RazorGeneratorResult(_project.Name, string.Join(Environment.NewLine, runResult.Diagnostics)));
250-
}
251-
252-
return null;
253-
}
254-
255-
return generatorResult;
256-
}
257201
}

‎src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteSnapshotManager.cs‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,17 @@ namespace Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem;
1414
internal sealed class RemoteSnapshotManager(IFilePathService filePathService, ITelemetryReporter telemetryReporter)
1515
{
1616
private static readonly ConditionalWeakTable<Solution, RemoteSolutionSnapshot> s_solutionToSnapshotMap = new();
17+
private static readonly object s_gate = new();
1718

1819
public IFilePathService FilePathService { get; } = filePathService;
1920
public ITelemetryReporter TelemetryReporter { get; } = telemetryReporter;
2021

2122
public RemoteSolutionSnapshot GetSnapshot(Solution solution)
2223
{
23-
return s_solutionToSnapshotMap.GetValue(solution, s => new RemoteSolutionSnapshot(s, this));
24+
lock (s_gate)
25+
{
26+
return s_solutionToSnapshotMap.GetValue(solution, s => new RemoteSolutionSnapshot(s, this));
27+
}
2428
}
2529

2630
public RemoteProjectSnapshot GetSnapshot(Project project)
@@ -32,4 +36,35 @@ public RemoteDocumentSnapshot GetSnapshot(TextDocument document)
3236
{
3337
return GetSnapshot(document.Project).GetDocument(document);
3438
}
39+
40+
internal Project? TryGetRetryProject(Project project)
41+
{
42+
if (project.IsRetryProject())
43+
{
44+
// If the passed in project is already a retry project, then it means whatever failure the caller had is real
45+
return null;
46+
}
47+
48+
lock (s_gate)
49+
{
50+
// Check if we already have performed a retry for this project. We only expect retry projects to be needed early in the life of a session,
51+
// so its highly likely the first few requests will all come in parallel (ie, inlay hints, folding ranges, semantic tokens) and well all
52+
// need to retry. This extra check means we don't create multiple retry projects for the same underlying project, and hence only run generators
53+
// once. Once almost anything in the project has changed, the source generator cache will be un-stuck, and this method won't be called, and
54+
// our retry solution snapshot will be removed from the CWT as normal.
55+
if (s_solutionToSnapshotMap.TryGetValue(project.Solution, out var snapshot) &&
56+
snapshot.GetProject(project) is { } existingProject &&
57+
existingProject.Project.IsRetryProject())
58+
{
59+
return existingProject.Project;
60+
}
61+
62+
// The passed in project isn't a retry, and we don't have one already, so create one now and replace
63+
// our current snapshot. This means future requests will just get the right thing from their first OOP service call.
64+
var retryProject = project.ForkToRetryProject();
65+
s_solutionToSnapshotMap.Remove(project.Solution);
66+
s_solutionToSnapshotMap.Add(project.Solution, new RemoteSolutionSnapshot(retryProject.Solution, this));
67+
return retryProject;
68+
}
69+
}
3570
}

‎src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteSolutionSnapshot.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,11 @@ public RemoteProjectSnapshot GetProject(Project project)
2828
{
2929
if (project.Solution != _solution)
3030
{
31-
throw new ArgumentException(SR.Project_does_not_belong_to_this_solution, nameof(project));
31+
// It might look like we don't know about this project, but we might have actually seen it before but been forced
32+
// to retry running source generators in it, so we handle that specific case here.
33+
project = _solution.GetProject(project.Id) is { } retryProject && retryProject.IsRetryProject()
34+
? retryProject
35+
: throw new ArgumentException(SR.Project_does_not_belong_to_this_solution, nameof(project));
3236
}
3337

3438
if (!project.ContainsRazorDocuments())

0 commit comments

Comments
 (0)