Skip to content

Commit

Permalink
Avoid cyclic references during incremental project update
Browse files Browse the repository at this point in the history
This change operates under the assumption that neither the starting
state nor the final state include any cycles. By removing all project
references from projects that change their reference sets prior to other
work, we ensure extraneous references cannot lead to an invalid
intermediate state.

This increases the amount of work required to complete an incremental
update when project references change, but does not impact the more
common updates where all project references are unchanged.

Fixes #52578
  • Loading branch information
sharwell committed Jan 7, 2022
1 parent 926ef57 commit c7673d6
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,54 @@ await solution.State.GetChecksumAsync(CancellationToken.None),
await remoteWorkspace.CurrentSolution.State.GetChecksumAsync(CancellationToken.None));
}

[Fact]
[WorkItem(52578, "https://github.com/dotnet/roslyn/issues/52578")]
public async Task TestIncrementalUpdateHandlesReferenceReversal()
{
using var workspace = CreateWorkspace();

using var client = await InProcRemoteHostClient.GetTestClientAsync(workspace).ConfigureAwait(false);
var remoteWorkspace = client.GetRemoteWorkspace();

var solution = workspace.CurrentSolution;
solution = AddProject(solution, LanguageNames.CSharp, documents: Array.Empty<string>(), additionalDocuments: Array.Empty<string>(), p2pReferences: Array.Empty<ProjectId>());
var projectId1 = solution.ProjectIds.Single();
solution = AddProject(solution, LanguageNames.CSharp, documents: Array.Empty<string>(), additionalDocuments: Array.Empty<string>(), p2pReferences: Array.Empty<ProjectId>());
var projectId2 = solution.ProjectIds.Where(id => id != projectId1).Single();

var project1ToProject2 = new ProjectReference(projectId2);
var project2ToProject1 = new ProjectReference(projectId1);

// Start with projectId1 -> projectId2
solution = solution.AddProjectReference(projectId1, project1ToProject2);

// verify initial setup
await UpdatePrimaryWorkspace(client, solution);
await VerifyAssetStorageAsync(client, solution, includeProjectCones: false);

Assert.Equal(
await solution.State.GetChecksumAsync(CancellationToken.None),
await remoteWorkspace.CurrentSolution.State.GetChecksumAsync(CancellationToken.None));

// reverse project references and incrementally update
solution = solution.RemoveProjectReference(projectId1, project1ToProject2);
solution = solution.AddProjectReference(projectId2, project2ToProject1);
await UpdatePrimaryWorkspace(client, solution);

Assert.Equal(
await solution.State.GetChecksumAsync(CancellationToken.None),
await remoteWorkspace.CurrentSolution.State.GetChecksumAsync(CancellationToken.None));

// reverse project references again and incrementally update
solution = solution.RemoveProjectReference(projectId2, project2ToProject1);
solution = solution.AddProjectReference(projectId1, project1ToProject2);
await UpdatePrimaryWorkspace(client, solution);

Assert.Equal(
await solution.State.GetChecksumAsync(CancellationToken.None),
await remoteWorkspace.CurrentSolution.State.GetChecksumAsync(CancellationToken.None));
}

[Fact]
public void TestRemoteWorkspaceCircularReferences()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,19 @@ private async Task<Solution> UpdateProjectsAsync(Solution solution, HashSet<Chec
}
}

// changed project
// remove all project references from projects that changed. this ensures exceptions will not occur for
// cyclic references during an incremental update.
foreach (var (projectId, newProjectChecksums) in newMap)
{
if (!oldMap.TryGetValue(projectId, out var oldProjectChecksums))
{
continue;
}

Contract.ThrowIfTrue(oldProjectChecksums.Checksum == newProjectChecksums.Checksum);

solution = await UpdateProjectAsync(solution.GetProject(projectId)!, oldProjectChecksums, newProjectChecksums).ConfigureAwait(false);
if (oldProjectChecksums.ProjectReferences.Checksum != newProjectChecksums.ProjectReferences.Checksum)
{
solution = solution.WithProjectReferences(projectId, SpecializedCollections.EmptyEnumerable<ProjectReference>());
}
}

// removed project
Expand All @@ -173,6 +175,19 @@ private async Task<Solution> UpdateProjectsAsync(Solution solution, HashSet<Chec
}
}

// changed project
foreach (var (projectId, newProjectChecksums) in newMap)
{
if (!oldMap.TryGetValue(projectId, out var oldProjectChecksums))
{
continue;
}

Contract.ThrowIfTrue(oldProjectChecksums.Checksum == newProjectChecksums.Checksum);

solution = await UpdateProjectAsync(solution.GetProject(projectId)!, oldProjectChecksums, newProjectChecksums).ConfigureAwait(false);
}

return solution;
}

Expand Down

0 comments on commit c7673d6

Please sign in to comment.