Skip to content

Commit f9090f4

Browse files
Handle concurrent requests to update workspace contents and workspace SG info. (#79628)
2 parents 3e0d00e + cb9aec5 commit f9090f4

File tree

16 files changed

+256
-65
lines changed

16 files changed

+256
-65
lines changed

src/EditorFeatures/Core/EditAndContinue/PdbMatchingSourceTextProvider.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ internal sealed class PdbMatchingSourceTextProvider() : IEventListener, IPdbMatc
2929
private readonly object _guard = new();
3030

3131
private bool _isActive;
32-
private int _baselineSolutionVersion;
32+
private int _baselineSolutionContentVersion;
3333
private readonly Dictionary<string, (DocumentState state, int solutionVersion)> _documentsWithChangedLoaderByPath = [];
3434
private WorkspaceEventRegistration? _workspaceChangedDisposer;
3535

@@ -84,12 +84,12 @@ private void WorkspaceChanged(WorkspaceChangeEventArgs e)
8484
// The file checksum is no longer available from the latter, so capture it at this moment.
8585
if (oldDocument.State.TextAndVersionSource.CanReloadText && !newDocument.State.TextAndVersionSource.CanReloadText)
8686
{
87-
var oldSolutionVersion = oldDocument.Project.Solution.WorkspaceVersion;
87+
var oldSolutionVersion = oldDocument.Project.Solution.SolutionStateContentVersion;
8888

8989
lock (_guard)
9090
{
9191
// ignore updates to a document that we have already seen this session:
92-
if (_isActive && oldSolutionVersion >= _baselineSolutionVersion && !_documentsWithChangedLoaderByPath.ContainsKey(oldDocument.FilePath))
92+
if (_isActive && oldSolutionVersion >= _baselineSolutionContentVersion && !_documentsWithChangedLoaderByPath.ContainsKey(oldDocument.FilePath))
9393
{
9494
_documentsWithChangedLoaderByPath.Add(oldDocument.FilePath, (oldDocument.DocumentState, oldSolutionVersion));
9595
}
@@ -104,7 +104,7 @@ public void SetBaseline(Solution solution)
104104
{
105105
lock (_guard)
106106
{
107-
_baselineSolutionVersion = solution.WorkspaceVersion;
107+
_baselineSolutionContentVersion = solution.SolutionStateContentVersion;
108108
}
109109
}
110110

src/EditorFeatures/Test/EditAndContinue/EditAndContinueLanguageServiceTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -395,15 +395,15 @@ public async Task DefaultPdbMatchingSourceTextProvider()
395395

396396
var (key, (documentState, version)) = sourceTextProvider.GetTestAccessor().GetDocumentsWithChangedLoaderByPath().Single();
397397
Assert.Equal(sourceFile.Path, key);
398-
Assert.Equal(solution.WorkspaceVersion, version);
399-
Assert.Equal(source1, (await documentState.GetTextAsync(CancellationToken.None)).ToString());
398+
Assert.Equal(solution.SolutionStateContentVersion, version);
399+
Assert.Equal(source1, documentState.GetTextSynchronously(CancellationToken.None).ToString());
400400

401401
// check committed document status:
402402
var debuggingSession = service.GetTestAccessor().GetActiveDebuggingSessions().Single();
403403
var (document, state) = await debuggingSession.LastCommittedSolution.GetDocumentAndStateAsync(document1, CancellationToken.None);
404404
var text = await document.GetTextAsync();
405405
Assert.Equal(CommittedSolution.DocumentState.MatchesBuildOutput, state);
406-
Assert.Equal(source1, (await document.GetTextAsync(CancellationToken.None)).ToString());
406+
Assert.Equal(source1, document.GetTextSynchronously(CancellationToken.None).ToString());
407407

408408
await languageService.EndSessionAsync(CancellationToken.None);
409409
}

src/Features/Core/Portable/Completion/Providers/CompletionUtilities.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public static ImmutableArray<Project> GetDistinctProjectsFromLatestSolutionSnaps
3838
foreach (var project in projects)
3939
{
4040
projectIds.Add(project.Id);
41-
if (solution is null || project.Solution.WorkspaceVersion > solution.WorkspaceVersion)
41+
if (solution is null || project.Solution.SolutionStateContentVersion > solution.SolutionStateContentVersion)
4242
{
4343
solution = project.Solution;
4444
}

src/LanguageServer/Protocol/Workspaces/LspWorkspaceManager.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
using Microsoft.CodeAnalysis.LanguageServer.Handler;
1414
using Microsoft.CodeAnalysis.LanguageServer.Handler.DocumentChanges;
1515
using Microsoft.CodeAnalysis.PooledObjects;
16-
using Microsoft.CodeAnalysis.Shared.Collections;
1716
using Microsoft.CodeAnalysis.Shared.Extensions;
1817
using Microsoft.CodeAnalysis.Text;
1918
using Microsoft.CommonLanguageServerProtocol.Framework;
@@ -53,7 +52,7 @@ internal sealed class LspWorkspaceManager : IDocumentChangeTracker, ILspService
5352
/// workspace).
5453
/// <para/> Access to this is guaranteed to be serial by the <see cref="RequestExecutionQueue{RequestContextType}"/>
5554
/// </summary>
56-
private readonly Dictionary<Workspace, (int? forkedFromVersion, Solution solution)> _cachedLspSolutions = [];
55+
private readonly Dictionary<Workspace, (int? forkedFromVersion, Checksum? sourceGeneratorChecksum, Solution solution)> _cachedLspSolutions = [];
5756

5857
/// <summary>
5958
/// Stores the current source text for each URI that is being tracked by LSP. Each time an LSP text sync
@@ -393,13 +392,20 @@ .. registeredWorkspaces.Where(workspace => workspace.Kind == WorkspaceKind.Misce
393392
if (doesAllTextMatch && doesAllSourceGeneratedTextMatch)
394393
{
395394
// Remember that the current LSP text matches the text in this workspace solution.
396-
_cachedLspSolutions[workspace] = (forkedFromVersion: null, workspaceCurrentSolution);
395+
_cachedLspSolutions[workspace] = (forkedFromVersion: null, sourceGeneratorChecksum: null, workspaceCurrentSolution);
397396
return (workspaceCurrentSolution, IsForked: false);
398397
}
399398

399+
var forkedFromVersion = workspaceCurrentSolution.SolutionStateContentVersion;
400+
var sourceGeneratorChecksum = workspaceCurrentSolution.CompilationState.SourceGeneratorExecutionVersionMap.GetChecksum();
401+
400402
// Step 4: See if we can reuse a previously forked solution.
401-
if (cachedSolution != default && cachedSolution.forkedFromVersion == workspaceCurrentSolution.WorkspaceVersion)
403+
if (cachedSolution != default &&
404+
cachedSolution.forkedFromVersion == forkedFromVersion &&
405+
cachedSolution.sourceGeneratorChecksum == sourceGeneratorChecksum)
406+
{
402407
return (cachedSolution.solution, IsForked: true);
408+
}
403409

404410
// Step 5: Fork a new solution from the workspace with the LSP text applied.
405411
var lspSolution = workspaceCurrentSolution;
@@ -417,7 +423,7 @@ .. registeredWorkspaces.Where(workspace => workspace.Kind == WorkspaceKind.Misce
417423
}
418424

419425
// Remember this forked solution and the workspace version it was forked from.
420-
_cachedLspSolutions[workspace] = (workspaceCurrentSolution.WorkspaceVersion, lspSolution);
426+
_cachedLspSolutions[workspace] = (forkedFromVersion, sourceGeneratorChecksum, lspSolution);
421427
return (lspSolution, IsForked: true);
422428
}
423429

src/LanguageServer/ProtocolUnitTests/Workspaces/LspWorkspaceManagerTests.cs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,9 @@
2020

2121
namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests.Workspaces;
2222

23-
public sealed class LspWorkspaceManagerTests : AbstractLanguageServerProtocolTests
23+
public sealed class LspWorkspaceManagerTests(ITestOutputHelper testOutputHelper)
24+
: AbstractLanguageServerProtocolTests(testOutputHelper)
2425
{
25-
public LspWorkspaceManagerTests(ITestOutputHelper testOutputHelper) : base(testOutputHelper)
26-
{
27-
}
28-
2926
[Theory, CombinatorialData]
3027
public async Task TestUsesLspTextOnOpenCloseAsync(bool mutatingLspWorkspace)
3128
{

src/LanguageServer/ProtocolUnitTests/Workspaces/SourceGeneratedDocumentTests.cs

Lines changed: 101 additions & 2 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
// See the LICENSE file in the project root for more information.
44

5+
using System.Diagnostics.CodeAnalysis;
56
using System.Linq;
67
using System.Threading;
78
using System.Threading.Tasks;
@@ -11,7 +12,6 @@
1112
using Microsoft.CodeAnalysis.Text;
1213
using Roslyn.Test.Utilities;
1314
using Roslyn.Test.Utilities.TestGenerators;
14-
using Roslyn.Utilities;
1515
using Xunit;
1616
using Xunit.Abstractions;
1717
using LSP = Roslyn.LanguageServer.Protocol;
@@ -223,6 +223,103 @@ internal async Task TestReturnsGeneratedSourceWhenManuallyRefreshed(bool mutatin
223223
Assert.Equal("// callCount: 1", secondRequest.Text);
224224
}
225225

226+
[Theory, CombinatorialData]
227+
internal async Task TestCanRunSourceGeneratorAndApplyChangesConcurrently(
228+
bool mutatingLspWorkspace,
229+
bool majorVersionUpdate,
230+
SourceGeneratorExecutionPreference sourceGeneratorExecution)
231+
{
232+
await using var testLspServer = await CreateTestLspServerAsync("""
233+
class C
234+
{
235+
}
236+
""", mutatingLspWorkspace);
237+
238+
var configService = testLspServer.TestWorkspace.ExportProvider.GetExportedValue<TestWorkspaceConfigurationService>();
239+
configService.Options = new WorkspaceConfigurationOptions(SourceGeneratorExecution: sourceGeneratorExecution);
240+
241+
var callCount = 0;
242+
var generatorReference = await AddGeneratorAsync(new CallbackGenerator(() => ("hintName.cs", "// callCount: " + callCount++)), testLspServer.TestWorkspace);
243+
244+
var sourceGeneratedDocuments = await testLspServer.GetCurrentSolution().Projects.Single().GetSourceGeneratedDocumentsAsync();
245+
var sourceGeneratedDocumentIdentity = sourceGeneratedDocuments.Single().Identity;
246+
var sourceGeneratorDocumentUri = SourceGeneratedDocumentUri.Create(sourceGeneratedDocumentIdentity);
247+
248+
var text = await testLspServer.ExecuteRequestAsync<SourceGeneratorGetTextParams, SourceGeneratedDocumentText>(SourceGeneratedDocumentGetTextHandler.MethodName,
249+
new SourceGeneratorGetTextParams(new LSP.TextDocumentIdentifier { DocumentUri = sourceGeneratorDocumentUri }, ResultId: null), CancellationToken.None);
250+
251+
AssertEx.NotNull(text);
252+
Assert.Equal("// callCount: 0", text.Text);
253+
254+
var initialSolution = testLspServer.GetCurrentSolution();
255+
var initialExecutionMap = initialSolution.CompilationState.SourceGeneratorExecutionVersionMap.Map;
256+
257+
// Updating the execution version should trigger source generators to run in both automatic and balanced mode.
258+
var forceRegeneration = majorVersionUpdate;
259+
testLspServer.TestWorkspace.EnqueueUpdateSourceGeneratorVersion(projectId: null, forceRegeneration);
260+
await testLspServer.WaitForSourceGeneratorsAsync();
261+
262+
var solutionWithChangedExecutionVersion = testLspServer.GetCurrentSolution();
263+
264+
var secondRequest = await testLspServer.ExecuteRequestAsync<SourceGeneratorGetTextParams, SourceGeneratedDocumentText>(SourceGeneratedDocumentGetTextHandler.MethodName,
265+
new SourceGeneratorGetTextParams(new LSP.TextDocumentIdentifier { DocumentUri = sourceGeneratorDocumentUri }, ResultId: text.ResultId), CancellationToken.None);
266+
AssertEx.NotNull(secondRequest);
267+
268+
if (forceRegeneration)
269+
{
270+
Assert.NotEqual(text.ResultId, secondRequest.ResultId);
271+
Assert.Equal("// callCount: 1", secondRequest.Text);
272+
}
273+
else
274+
{
275+
Assert.Equal(text.ResultId, secondRequest.ResultId);
276+
Assert.Null(secondRequest.Text);
277+
}
278+
279+
var projectId1 = initialSolution.ProjectIds.Single();
280+
var solutionWithDocumentChanged = initialSolution.WithDocumentText(
281+
initialSolution.Projects.Single().Documents.Single().Id,
282+
SourceText.From("class D { }"));
283+
284+
var expectVersionChange = sourceGeneratorExecution is SourceGeneratorExecutionPreference.Balanced || forceRegeneration;
285+
286+
// The content forked solution should have an SG execution version *less than* the one we just changed.
287+
// Note: this will be patched up once we call TryApplyChanges.
288+
if (expectVersionChange)
289+
{
290+
Assert.True(
291+
solutionWithChangedExecutionVersion.CompilationState.SourceGeneratorExecutionVersionMap[projectId1]
292+
> solutionWithDocumentChanged.CompilationState.SourceGeneratorExecutionVersionMap[projectId1]);
293+
}
294+
else
295+
{
296+
Assert.Equal(
297+
solutionWithChangedExecutionVersion.CompilationState.SourceGeneratorExecutionVersionMap[projectId1],
298+
solutionWithDocumentChanged.CompilationState.SourceGeneratorExecutionVersionMap[projectId1]);
299+
}
300+
301+
Assert.True(testLspServer.TestWorkspace.TryApplyChanges(solutionWithDocumentChanged));
302+
303+
var finalSolution = testLspServer.GetCurrentSolution();
304+
305+
if (expectVersionChange)
306+
{
307+
// In balanced (or if we forced regen) mode, the execution version should have been updated to the new value.
308+
Assert.NotEqual(initialExecutionMap[projectId1], solutionWithChangedExecutionVersion.CompilationState.SourceGeneratorExecutionVersionMap[projectId1]);
309+
Assert.NotEqual(initialExecutionMap[projectId1], finalSolution.CompilationState.SourceGeneratorExecutionVersionMap[projectId1]);
310+
}
311+
else
312+
{
313+
// In automatic mode, nothing should change wrt to execution versions (unless we specified force-regenerate).
314+
Assert.Equal(initialExecutionMap[projectId1], solutionWithChangedExecutionVersion.CompilationState.SourceGeneratorExecutionVersionMap[projectId1]);
315+
Assert.Equal(initialExecutionMap[projectId1], finalSolution.CompilationState.SourceGeneratorExecutionVersionMap[projectId1]);
316+
}
317+
318+
// The final execution version for the project should match the changed execution version, no matter what.
319+
// Proving that the content change happened, but didn't drop the execution version change.
320+
Assert.Equal(solutionWithChangedExecutionVersion.CompilationState.SourceGeneratorExecutionVersionMap[projectId1], finalSolution.CompilationState.SourceGeneratorExecutionVersionMap[projectId1]);
321+
}
322+
226323
[Theory, CombinatorialData]
227324
public async Task TestReturnsNullForRemovedClosedGeneratedFile(bool mutatingLspWorkspace)
228325
{
@@ -279,7 +376,9 @@ public async Task TestReturnsNullForRemovedOpenedGeneratedFile(bool mutatingLspW
279376
Assert.Null(secondRequest.Text);
280377
}
281378

282-
private async Task<TestLspServer> CreateTestLspServerWithGeneratorAsync(bool mutatingLspWorkspace, string generatedDocumentText)
379+
private async Task<TestLspServer> CreateTestLspServerWithGeneratorAsync(
380+
bool mutatingLspWorkspace,
381+
[StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string generatedDocumentText)
283382
{
284383
var testLspServer = await CreateTestLspServerAsync(string.Empty, mutatingLspWorkspace);
285384
await AddGeneratorAsync(new SingleFileTestGenerator(generatedDocumentText), testLspServer.TestWorkspace);

src/Tools/ExternalAccess/Razor/Features/SolutionExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@ public static ImmutableArray<DocumentId> GetDocumentIds(this Solution solution,
1616
=> LanguageServer.Extensions.GetDocumentIds(solution, new(documentUri));
1717

1818
public static int GetWorkspaceVersion(this Solution solution)
19-
=> solution.WorkspaceVersion;
19+
=> solution.SolutionStateContentVersion;
2020
}

src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/ITextViewWindowVerifierInProcessExtensions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,11 @@ await textViewWindowVerifier.TestServices.Workspace.WaitForAllAsyncOperationsAsy
115115
],
116116
cancellationToken);
117117

118-
if (codeActionLogger.Messages.Any())
118+
if (codeActionLogger.Messages.Count != 0)
119119
{
120120
foreach (var e in events)
121121
{
122-
codeActionLogger.Messages.Add($"{e.OldSolution.WorkspaceVersion} to {e.NewSolution.WorkspaceVersion}: {e.Kind} {e.DocumentId}");
122+
codeActionLogger.Messages.Add($"{e.OldSolution.SolutionStateContentVersion} to {e.NewSolution.SolutionStateContentVersion}: {e.Kind} {e.DocumentId}");
123123
}
124124
}
125125

src/Workspaces/Core/Portable/CodeActions/Operations/ApplyChangesOperation.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ internal static bool ApplyOrMergeChanges(
5353
var currentSolution = workspace.CurrentSolution;
5454

5555
// if there was no intermediary edit, just apply the change fully.
56-
if (changedSolution.WorkspaceVersion == currentSolution.WorkspaceVersion)
56+
if (changedSolution.SolutionStateContentVersion == currentSolution.SolutionStateContentVersion)
5757
{
5858
var result = workspace.TryApplyChanges(changedSolution, progressTracker);
5959

src/Workspaces/Core/Portable/ExternalAccess/UnitTesting/Api/UnitTestingSolutionExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace Microsoft.CodeAnalysis.ExternalAccess.UnitTesting.Api;
1010
internal static class UnitTestingSolutionExtensions
1111
{
1212
public static int GetWorkspaceVersion(this Solution solution)
13-
=> solution.WorkspaceVersion;
13+
=> solution.SolutionStateContentVersion;
1414

1515
public static async Task<UnitTestingChecksumWrapper> GetChecksumAsync(this Solution solution, CancellationToken cancellationToken)
1616
=> new UnitTestingChecksumWrapper(await solution.CompilationState.GetChecksumAsync(cancellationToken).ConfigureAwait(false));

0 commit comments

Comments
 (0)