Skip to content

Commit

Permalink
When we're removing the last project, fire SolutionRemoved
Browse files Browse the repository at this point in the history
We have a fair bit of code that assumed it could look for a
WorkspaceChangeKind of SolutionRemoved or SolutionCleared to indicate
when we've closed the solution and can drop caches and such. Except we
never actually fired anything like that. This fires SolutionRemoved on
the removal of the last project.

We had a test that asserted that the solution persistence service would
continue to return the solution path even once the solution was closed;
this fixes that test to assert that it's no longer available.
  • Loading branch information
jasonmalinowski committed Jul 11, 2022
1 parent e9da5cf commit e443265
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 7 deletions.
12 changes: 11 additions & 1 deletion src/VisualStudio/Core/Def/ProjectSystem/VisualStudioProject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,17 @@ public void RemoveFromWorkspace()
// references being converted to metadata references (or vice versa) and we might either
// miss stopping a file watcher or might end up double-stopping a file watcher.
remainingMetadataReferences = w.CurrentSolution.GetRequiredProject(Id).MetadataReferences;
w.OnProjectRemoved(Id);
_workspace.RemoveProjectFromTrackingMaps_NoLock(Id);
// If this is our last project, clear the entire solution.
if (w.CurrentSolution.ProjectIds.Count == 1)
{
_workspace.RemoveSolution_NoLock();
}
else
{
_workspace.OnProjectRemoved(Id);
}
});

Contract.ThrowIfNull(remainingMetadataReferences);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.ExternalAccess.VSTypeScript.Api;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.VisualStudio.LanguageServices.ExternalAccess.VSTypeScript.Api;
Expand Down Expand Up @@ -115,6 +116,7 @@ await _visualStudioWorkspaceImpl.ApplyChangeToWorkspaceAsync(w =>
.WithTelemetryId(creationInfo.ProjectGuid);
// If we don't have any projects and this is our first project being added, then we'll create a new SolutionId
// and count this as the solution being added so that event is raised.
if (w.CurrentSolution.ProjectIds.Count == 0)
{
var solutionSessionId = GetSolutionSessionId();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1667,7 +1667,11 @@ private ProjectReferenceInformation GetReferenceInfo_NoLock(ProjectId projectId)
return _projectReferenceInfoMap.GetOrAdd(projectId, _ => new ProjectReferenceInformation());
}

protected internal override void OnProjectRemoved(ProjectId projectId)
/// <summary>
/// Removes the project from the various maps this type maintains; it's still up to the caller to actually remove
/// the project in one way or another.
/// </summary>
internal void RemoveProjectFromTrackingMaps_NoLock(ProjectId projectId)
{
string? languageName;

Expand Down Expand Up @@ -1711,8 +1715,6 @@ protected internal override void OnProjectRemoved(ProjectId projectId)
}
}

base.OnProjectRemoved(projectId);

// Try to update the UI context info. But cancel that work if we're shutting down.
_threadingContext.RunWithShutdownBlockAsync(async cancellationToken =>
{
Expand All @@ -1721,6 +1723,31 @@ protected internal override void OnProjectRemoved(ProjectId projectId)
});
}

internal void RemoveSolution_NoLock()
{
Contract.ThrowIfFalse(_gate.CurrentCount == 0);

// At this point, we should have had RemoveProjectFromTrackingMaps_NoLock called for everything else, so it's just the solution itself
// to clean up
Contract.ThrowIfFalse(_projectReferenceInfoMap.Count == 0);
Contract.ThrowIfFalse(_projectToHierarchyMap.Count == 0);
Contract.ThrowIfFalse(_projectToGuidMap.Count == 0);
Contract.ThrowIfFalse(_projectToMaxSupportedLangVersionMap.Count == 0);
Contract.ThrowIfFalse(_projectToDependencyNodeTargetIdentifier.Count == 0);
Contract.ThrowIfFalse(_projectToRuleSetFilePath.Count == 0);
Contract.ThrowIfFalse(_projectSystemNameToProjectsMap.Count == 0);

// Create a new empty solution and set this; we will reuse the same SolutionId and path since components still may have persistence information they still need
// to look up by that location; we also keep the existing analyzer references around since those are host-level analyzers that were loaded asynchronously.
SetCurrentSolution(
solution => CreateSolution(
SolutionInfo.Create(
SolutionId.CreateNewId(),
VersionStamp.Create(),
analyzerReferences: solution.AnalyzerReferences)),
WorkspaceChangeKind.SolutionRemoved);
}

private sealed class ProjectReferenceInformation
{
public readonly List<string> OutputPaths = new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,5 +106,21 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
Assert.Same(startingSolution, environment.Workspace.CurrentSolution)
End Using
End Function

<WpfFact>
<CombinatorialData>
Public Async Function AddingAndRemovingOnlyProjectTriggersSolutionAddedAndSolutionRemoved() As Task
Using environment = New TestEnvironment()
Dim workspaceChangeEvents = New WorkspaceChangeWatcher(environment)
Dim project = Await environment.ProjectFactory.CreateAndAddToWorkspaceAsync(
"Project", LanguageNames.CSharp, CancellationToken.None)

Assert.Equal(WorkspaceChangeKind.SolutionAdded, Assert.Single(Await workspaceChangeEvents.GetNewChangeEventsAsync()).Kind)

project.RemoveFromWorkspace()

Assert.Equal(WorkspaceChangeKind.SolutionRemoved, Assert.Single(Await workspaceChangeEvents.GetNewChangeEventsAsync()).Kind)
End Using
End Function
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ public async Task VerifyWorkingFolder()

await TestServices.SolutionExplorer.CloseSolutionAsync(HangMitigatingCancellationToken);

// because the solution cache directory is stored in the user temp folder,
// closing the solution has no effect on what is returned.
Assert.NotNull(persistentStorageConfiguration.TryGetStorageLocation(SolutionKey.ToSolutionKey(visualStudioWorkspace.CurrentSolution)));
// Since we no longer have an open solution, we don't have a storage location for it, since that
// depends on the open solution.
Assert.Null(persistentStorageConfiguration.TryGetStorageLocation(SolutionKey.ToSolutionKey(visualStudioWorkspace.CurrentSolution)));
}

private async Task WaitForNavigateAsync(CancellationToken cancellationToken)
Expand Down

0 comments on commit e443265

Please sign in to comment.