From 487fb6d90e214a77c79789fdef90954bc7296b86 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 11 Oct 2026 11:45:51 +0200 Subject: [PATCH 1/5] Release project snapshots when their project leaves the workspace With the transparent compiler on, the editor keeps the last snapshot of every F# project it has built one for, and a memoized snapshot per Roslyn project version. Each holds the Roslyn `Project` it was built from, and through it the whole `Solution`. Nothing removed them: the dictionary of last snapshots only grew, so every solution opened in a session stayed reachable until Visual Studio exited. Both stores now live in ProjectSnapshotCache and follow the workspace. A project's entries go when the workspace reports it removed, the old solution's projects go when the solution is removed, cleared or reloaded, and closing the solution clears everything, next to the other caches. A snapshot requested for a project that has already left is stored first and checked after, so late work cannot bring an entry back. Co-Authored-By: Claude Opus 5.5 --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + .../LanguageService/LanguageService.fs | 12 ++++ .../LanguageService/WorkspaceExtensions.fs | 57 ++++++++++++----- .../FSharp.Editor.Tests.fsproj | 1 + .../ProjectSnapshotCacheTests.fs | 61 +++++++++++++++++++ 5 files changed, 117 insertions(+), 15 deletions(-) create mode 100644 vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 2357fbcdff4..62256b4f982 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -21,6 +21,7 @@ * Improve static compilation of state machines. ([PR #19297](https://github.com/dotnet/fsharp/pull/19297)) * Make Alt+F1 (momentary toggle) work for inlay hints. ([PR #19421](https://github.com/dotnet/fsharp/pull/19421)) * Fix doubled F# diagnostics in tooltips. ([Issue #16360](https://github.com/dotnet/fsharp/issues/16360)) +* Release the project snapshots the editor keeps for the transparent compiler when their project is removed or the solution is closed. Each one held the Roslyn solution it was built from, so every solution opened in a session stayed in memory until Visual Studio exited. * Fix `NotSupportedException` in the memory-mapped-file optimization when copying `ReadOnlyMemory` into `MemoryMappedFileViewStream`. ([Issue #20263](https://github.com/dotnet/fsharp/issues/20263)) * Reduce allocations in the VS project options reactor: the command-line options and project options caches and the mailbox reply payloads now hold struct tuples, and `IProjectSite.CompilationBinOutputPath` returns `string voption` picked with a new `Array.tryPickV`. ([PR #20413](https://github.com/dotnet/fsharp/pull/20413)) * Build a single-file project's `OtherOptions` reference flags with one array comprehension instead of two `Array.ofSeq` calls and an `Array.append`. ([PR #20499](https://github.com/dotnet/fsharp/pull/20499)) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs index ab876679a77..ccc493dfed3 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs @@ -220,6 +220,17 @@ type internal FSharpWorkspaceServiceFactory let optionsManager = FSharpProjectOptionsManager(checker, workspace) + // The snapshot caches hold the Roslyn project, and through it the solution, of every snapshot they keep. + workspace.WorkspaceChanged.Add(fun args -> + match args.Kind with + | WorkspaceChangeKind.ProjectRemoved -> ProjectSnapshotCache.removeProject args.ProjectId + | WorkspaceChangeKind.SolutionRemoved + | WorkspaceChangeKind.SolutionCleared + | WorkspaceChangeKind.SolutionReloaded -> + for projectId in args.OldSolution.ProjectIds do + ProjectSnapshotCache.removeProject projectId + | _ -> ()) + { new IFSharpWorkspaceService with member _.Checker = checker member _.FSharpProjectOptionsManager = optionsManager @@ -235,6 +246,7 @@ type private FSharpSolutionEvents(projectManager: FSharpProjectOptionsManager, m projectManager.Checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients() metadataAsSource.ClearGeneratedFiles() projectManager.ClearAllCaches() + ProjectSnapshotCache.clear () VSConstants.S_OK member _.OnAfterLoadProject(_, _) = VSConstants.E_NOTIMPL diff --git a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs index 2406f3a6e32..a9f3e5be1bf 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs @@ -161,12 +161,29 @@ module internal FSharpProjectSnapshotSerialization = open FSharpProjectSnapshotSerialization open System.Collections.Concurrent -[] -module private CheckerExtensions = +/// The project snapshots built for the transparent compiler, kept per Roslyn project. Every entry holds the +/// `Project` it was built from, and through it the whole `Solution`, and `latest` is never trimmed, so an +/// entry has to go when its project leaves the workspace. +[] +module internal ProjectSnapshotCache = + + let snapshots = + AsyncMemoize(1000, 500, "SnapshotCache") + + /// What the last snapshot of each project was built from, for the next one to reuse. + let latest = + ConcurrentDictionary * FSharpProjectOptions * FSharpProjectSnapshot>() - let snapshotCache = AsyncMemoize(1000, 500, "SnapshotCache") + let removeProject (projectId: ProjectId) = + snapshots.Clear(fun key -> key = projectId) + latest.TryRemove projectId |> ignore - let latestSnapshots = ConcurrentDictionary<_, _>() + let clear () = + snapshots.Clear() + latest.Clear() + +[] +module private CheckerExtensions = let exist xs = xs |> Seq.isEmpty |> not @@ -241,7 +258,7 @@ module private CheckerExtensions = let! referenceVersions = getReferencedProjectVersions project let updatedSnapshot = - match project.IsTransparentCompilerSnapshotReuseEnabled, latestSnapshots.TryGetValue project.Id with + match project.IsTransparentCompilerSnapshotReuseEnabled, ProjectSnapshotCache.latest.TryGetValue project.Id with | true, (true, (_, _, oldReferenceVersions, _, _)) when referenceVersions <> oldReferenceVersions -> System.Diagnostics.Trace.TraceWarning "Reference versions changed" None @@ -362,7 +379,7 @@ module private CheckerExtensions = let latestSnapshotData = project, projectVersion, referenceVersions, options, newSnapshot - latestSnapshots.AddOrUpdate(project.Id, latestSnapshotData, (fun _ _ -> latestSnapshotData)) + ProjectSnapshotCache.latest.AddOrUpdate(project.Id, latestSnapshotData, (fun _ _ -> latestSnapshotData)) |> ignore return newSnapshot @@ -377,16 +394,26 @@ module private CheckerExtensions = member _.GetLabel() = project.FilePath } - snapshotCache.Get( - key, - async { - let! ct = Async.CancellationToken + async { + let! snapshot = + ProjectSnapshotCache.snapshots.Get( + key, + async { + let! ct = Async.CancellationToken - return! - createProjectSnapshot snapshotAccumulatorOpt project options ct - |> Async.AwaitTask - } - ) + return! + createProjectSnapshot snapshotAccumulatorOpt project options ct + |> Async.AwaitTask + } + ) + + // Stored first and checked after: a project that left the workspace while its snapshot was being + // built is dropped here, or by the workspace's own handler if it leaves after this check. + if not (project.Solution.Workspace.CurrentSolution.ContainsProject project.Id) then + ProjectSnapshotCache.removeProject project.Id + + return snapshot + } let getProjectSnapshotForDocument (document: Document, options: FSharpProjectOptions) = getOrCreateSnapshotForProject document.Project (Some options) None diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index d5d1a09c79a..0e881ad7a77 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -89,6 +89,7 @@ + diff --git a/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs b/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs new file mode 100644 index 00000000000..3eb258bee25 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +/// The snapshot caches hold the Roslyn project each snapshot was built from, so they have to follow the +/// workspace: a project that leaves it takes its entries along. +module FSharp.Editor.Tests.ProjectSnapshotCacheTests + +open System +open System.Threading +open System.Threading.Tasks +open Xunit +open Microsoft.CodeAnalysis +open Microsoft.VisualStudio.FSharp.Editor +open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks +open FSharp.Editor.Tests.Helpers + +let private createProject () = + RoslynTestHelpers.CreateSolution("module Test").Projects |> Seq.exactlyOne + +let private snapshotOf (project: Project) = + project.GetFSharpProjectSnapshot() + |> CancellableTask.start CancellationToken.None + +/// Removes the project and waits for the caches to hear of it: the workspace raises its change events on a +/// queue of its own. +let private removeFromWorkspace (project: Project) = + let workspace = project.Solution.Workspace + Assert.True(workspace.TryApplyChanges(workspace.CurrentSolution.RemoveProject project.Id)) + + Assert.True(SpinWait.SpinUntil((fun () -> not (ProjectSnapshotCache.latest.ContainsKey project.Id)), TimeSpan.FromSeconds 10.)) + +[] +let ``A project's snapshot is forgotten when the project leaves the workspace`` () : Task = + task { + let project = createProject () + + let! first = snapshotOf project + let! again = snapshotOf project + Assert.Same(first, again) + Assert.True(ProjectSnapshotCache.latest.ContainsKey project.Id) + + removeFromWorkspace project + + // Asked again, the same Roslyn project gets a snapshot built anew: the one it had is gone. + let! afterRemoval = snapshotOf project + Assert.NotSame(first, afterRemoval) + } + +[] +let ``A snapshot built for a project that has left the workspace is not kept`` () : Task = + task { + let project = createProject () + let! _ = snapshotOf project + removeFromWorkspace project + + // Work under way for a project when it leaves still finishes, and must not bring its entries back. + let! late = snapshotOf project + Assert.False(ProjectSnapshotCache.latest.ContainsKey project.Id) + + let! later = snapshotOf project + Assert.NotSame(late, later) + } From e1be7be35b07ae10c42af560d4355b25f343d243 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 11 Oct 2026 11:53:33 +0200 Subject: [PATCH 2/5] Link the release note to PR #20762 Co-Authored-By: Claude Opus 5.5 --- docs/release-notes/.VisualStudio/18.vNext.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 62256b4f982..e7685c41707 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -21,7 +21,7 @@ * Improve static compilation of state machines. ([PR #19297](https://github.com/dotnet/fsharp/pull/19297)) * Make Alt+F1 (momentary toggle) work for inlay hints. ([PR #19421](https://github.com/dotnet/fsharp/pull/19421)) * Fix doubled F# diagnostics in tooltips. ([Issue #16360](https://github.com/dotnet/fsharp/issues/16360)) -* Release the project snapshots the editor keeps for the transparent compiler when their project is removed or the solution is closed. Each one held the Roslyn solution it was built from, so every solution opened in a session stayed in memory until Visual Studio exited. +* Release the project snapshots the editor keeps for the transparent compiler when their project is removed or the solution is closed. Each one held the Roslyn solution it was built from, so every solution opened in a session stayed in memory until Visual Studio exited. ([PR #20762](https://github.com/dotnet/fsharp/pull/20762)) * Fix `NotSupportedException` in the memory-mapped-file optimization when copying `ReadOnlyMemory` into `MemoryMappedFileViewStream`. ([Issue #20263](https://github.com/dotnet/fsharp/issues/20263)) * Reduce allocations in the VS project options reactor: the command-line options and project options caches and the mailbox reply payloads now hold struct tuples, and `IProjectSite.CompilationBinOutputPath` returns `string voption` picked with a new `Array.tryPickV`. ([PR #20413](https://github.com/dotnet/fsharp/pull/20413)) * Build a single-file project's `OtherOptions` reference flags with one array comprehension instead of two `Array.ofSeq` calls and an `Array.append`. ([PR #20499](https://github.com/dotnet/fsharp/pull/20499)) From 51a6a0a8e9be4c93b742adbb87af0751a91849a4 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 11 Oct 2026 12:21:31 +0200 Subject: [PATCH 3/5] Give the workspace's change event a full minute to arrive in the test The wait is for an event the workspace raises from the thread pool. In a full run of the editor tests the pool can take longer than ten seconds to start queued work. Co-Authored-By: Claude Opus 5.5 --- .../tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs b/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs index 3eb258bee25..b5d1df96fa6 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs @@ -26,7 +26,7 @@ let private removeFromWorkspace (project: Project) = let workspace = project.Solution.Workspace Assert.True(workspace.TryApplyChanges(workspace.CurrentSolution.RemoveProject project.Id)) - Assert.True(SpinWait.SpinUntil((fun () -> not (ProjectSnapshotCache.latest.ContainsKey project.Id)), TimeSpan.FromSeconds 10.)) + Assert.True(SpinWait.SpinUntil((fun () -> not (ProjectSnapshotCache.latest.ContainsKey project.Id)), TimeSpan.FromSeconds 60.)) [] let ``A project's snapshot is forgotten when the project leaves the workspace`` () : Task = From adb3504e6394ec784d049087f63aad9fb543ceb0 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 15 Oct 2026 02:12:07 +0100 Subject: [PATCH 4/5] Tell a removal during a build by counting removals, not by asking for the id A snapshot being built when its project left was dropped afterwards if the workspace no longer held the project's id. A reload brings the same ids back, so the check let the build of the old solution's project keep its entries, and with them the old Project and its solution, until the next snapshot of that id replaced them. The cache now counts the removals the workspace reports. A request notes the count when it starts and, when it moved by the time the build ends, forgets the project's entries; the next request builds them again. The tests hold the request on the text of the document, a loader the test releases, and remove the project, or clear the solution and add it again under the same ids, while the request is pending. With the id check the reload test fails. Co-Authored-By: Claude Sonnet 5.5 --- .../LanguageService/WorkspaceExtensions.fs | 26 ++++- .../ProjectSnapshotCacheTests.fs | 100 ++++++++++++++++-- 2 files changed, 111 insertions(+), 15 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs index a9f3e5be1bf..42a165eeb7b 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs @@ -174,11 +174,24 @@ module internal ProjectSnapshotCache = let latest = ConcurrentDictionary * FSharpProjectOptions * FSharpProjectSnapshot>() - let removeProject (projectId: ProjectId) = + let mutable private removals = 0 + + /// How many removals the workspace has reported so far. A request that finds it moved while it was under + /// way may have built a snapshot of a project the workspace dropped, and the project's id cannot tell: + /// a reload brings the same ids back. + let removalsSoFar () = + System.Threading.Volatile.Read &removals + + let forget (projectId: ProjectId) = snapshots.Clear(fun key -> key = projectId) latest.TryRemove projectId |> ignore + let removeProject (projectId: ProjectId) = + System.Threading.Interlocked.Increment &removals |> ignore + forget projectId + let clear () = + System.Threading.Interlocked.Increment &removals |> ignore snapshots.Clear() latest.Clear() @@ -395,6 +408,8 @@ module private CheckerExtensions = } async { + let removalsAtStart = ProjectSnapshotCache.removalsSoFar () + let! snapshot = ProjectSnapshotCache.snapshots.Get( key, @@ -407,10 +422,11 @@ module private CheckerExtensions = } ) - // Stored first and checked after: a project that left the workspace while its snapshot was being - // built is dropped here, or by the workspace's own handler if it leaves after this check. - if not (project.Solution.Workspace.CurrentSolution.ContainsProject project.Id) then - ProjectSnapshotCache.removeProject project.Id + // The handler that follows the workspace may have run before this request stored its entries, and a + // reload brings the project's id back, so the id says nothing: if a removal happened meanwhile, this + // project's entries go, and the next request builds them again. + if ProjectSnapshotCache.removalsSoFar () <> removalsAtStart then + ProjectSnapshotCache.forget project.Id return snapshot } diff --git a/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs b/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs index b5d1df96fa6..77dc08672e2 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs @@ -1,7 +1,8 @@ // Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. /// The snapshot caches hold the Roslyn project each snapshot was built from, so they have to follow the -/// workspace: a project that leaves it takes its entries along. +/// workspace: a project that leaves it takes its entries along, also when it leaves while a snapshot of it is +/// being built. module FSharp.Editor.Tests.ProjectSnapshotCacheTests open System @@ -9,24 +10,62 @@ open System.Threading open System.Threading.Tasks open Xunit open Microsoft.CodeAnalysis +open Microsoft.CodeAnalysis.Text open Microsoft.VisualStudio.FSharp.Editor open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks open FSharp.Editor.Tests.Helpers +/// Text that arrives only when the test lets it, so a snapshot request that needs it stays pending, and that +/// says when a request has got as far as asking for it. +type private GatedTextLoader(text: string, release: Task) = + inherit TextLoader() + + let asked = + TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) + + let loaded () = + TextAndVersion.Create(SourceText.From text, VersionStamp.Create()) + + member _.Asked = asked.Task + + override _.LoadTextAndVersionAsync(_options: LoadTextOptions, _cancellationToken: CancellationToken) : Task = + task { + asked.TrySetResult() |> ignore + do! release + return loaded () + } + let private createProject () = RoslynTestHelpers.CreateSolution("module Test").Projects |> Seq.exactlyOne +/// A project whose only document gets its text when `release` completes. +let private createProjectWithGatedText (release: Task) = + let projectId = ProjectId.CreateNewId() + let loader = GatedTextLoader("module Test", release) + + let document = + DocumentInfo.Create(DocumentId.CreateNewId projectId, "test.fs", loader = loader, filePath = "C:\\test.fs") + + let projectInfo = + RoslynTestHelpers.CreateProjectInfo projectId "C:\\test.fsproj" [ document ] + + let solution = RoslynTestHelpers.CreateSolution [ projectInfo ] + RoslynTestHelpers.SetProjectOptions projectId solution RoslynTestHelpers.DefaultProjectOptions + loader, projectInfo, solution.GetProject projectId + let private snapshotOf (project: Project) = project.GetFSharpProjectSnapshot() |> CancellableTask.start CancellationToken.None -/// Removes the project and waits for the caches to hear of it: the workspace raises its change events on a -/// queue of its own. +/// The workspace raises its change events on a queue of its own: waits until the caches have heard of a removal. +let private waitForRemovalAfter (removalsBefore: int) = + Assert.True(SpinWait.SpinUntil((fun () -> ProjectSnapshotCache.removalsSoFar () <> removalsBefore), TimeSpan.FromSeconds 60.)) + let private removeFromWorkspace (project: Project) = let workspace = project.Solution.Workspace + let before = ProjectSnapshotCache.removalsSoFar () Assert.True(workspace.TryApplyChanges(workspace.CurrentSolution.RemoveProject project.Id)) - - Assert.True(SpinWait.SpinUntil((fun () -> not (ProjectSnapshotCache.latest.ContainsKey project.Id)), TimeSpan.FromSeconds 60.)) + waitForRemovalAfter before [] let ``A project's snapshot is forgotten when the project leaves the workspace`` () : Task = @@ -39,6 +78,8 @@ let ``A project's snapshot is forgotten when the project leaves the workspace`` Assert.True(ProjectSnapshotCache.latest.ContainsKey project.Id) removeFromWorkspace project + // The handler counts the removal first and forgets after: wait for the entries to be gone. + Assert.True(SpinWait.SpinUntil((fun () -> not (ProjectSnapshotCache.latest.ContainsKey project.Id)), TimeSpan.FromSeconds 60.)) // Asked again, the same Roslyn project gets a snapshot built anew: the one it had is gone. let! afterRemoval = snapshotOf project @@ -46,14 +87,53 @@ let ``A project's snapshot is forgotten when the project leaves the workspace`` } [] -let ``A snapshot built for a project that has left the workspace is not kept`` () : Task = +let ``A snapshot being built when its project leaves the workspace is not kept`` () : Task = task { - let project = createProject () - let! _ = snapshotOf project + let release = + TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) + + let loader, _, project = createProjectWithGatedText release.Task + + // On a pool thread: a request starts on its caller's, which has to be free to let the text go. + let pending = Task.Run(Func>(fun () -> snapshotOf project)) + do! loader.Asked + removeFromWorkspace project + release.SetResult() + + let! late = pending + Assert.False(ProjectSnapshotCache.latest.ContainsKey project.Id) + + // Nothing memoized either: the same Roslyn project gets a snapshot built anew. + let! later = snapshotOf project + Assert.NotSame(late, later) + } + +[] +let ``A snapshot being built when the solution is reloaded is not kept`` () : Task = + task { + let release = + TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) + + let loader, projectInfo, project = createProjectWithGatedText release.Task + let workspace = project.Solution.Workspace :?> AdhocWorkspace + + let pending = Task.Run(Func>(fun () -> snapshotOf project)) + do! loader.Asked + + // A reload brings the same project id back, so a request that asked the workspace whether the id is + // still there would keep what it built for the project that went. + let before = ProjectSnapshotCache.removalsSoFar () + workspace.ClearSolution() + + workspace.AddSolution(SolutionInfo.Create(SolutionId.CreateNewId(), VersionStamp.Create(), "test.sln", [ projectInfo ])) + |> ignore + + waitForRemovalAfter before + Assert.True(workspace.CurrentSolution.ContainsProject project.Id) - // Work under way for a project when it leaves still finishes, and must not bring its entries back. - let! late = snapshotOf project + release.SetResult() + let! late = pending Assert.False(ProjectSnapshotCache.latest.ContainsKey project.Id) let! later = snapshotOf project From effe5fde525cccac46107d9026d653336b967c45 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 18 Oct 2026 03:31:29 +0200 Subject: [PATCH 5/5] Forget a project's snapshots when its request is cancelled or starts after the removal The forget that follows a removal ran only when the request completed, and a request begun after the project left the workspace stored its snapshot again. The forget now runs in a finally block, the store checks the workspace first, and the comments are replaced by the names that carry what they said. Co-Authored-By: Claude Sonnet 5.5 --- .../LanguageService/LanguageService.fs | 1 - .../LanguageService/WorkspaceExtensions.fs | 53 ++++----- .../ProjectSnapshotCacheTests.fs | 112 ++++++++---------- 3 files changed, 73 insertions(+), 93 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs index ccc493dfed3..c86c5ca27f1 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs @@ -220,7 +220,6 @@ type internal FSharpWorkspaceServiceFactory let optionsManager = FSharpProjectOptionsManager(checker, workspace) - // The snapshot caches hold the Roslyn project, and through it the solution, of every snapshot they keep. workspace.WorkspaceChanged.Add(fun args -> match args.Kind with | WorkspaceChangeKind.ProjectRemoved -> ProjectSnapshotCache.removeProject args.ProjectId diff --git a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs index 42a165eeb7b..0ed5c8ef14c 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/WorkspaceExtensions.fs @@ -161,24 +161,18 @@ module internal FSharpProjectSnapshotSerialization = open FSharpProjectSnapshotSerialization open System.Collections.Concurrent -/// The project snapshots built for the transparent compiler, kept per Roslyn project. Every entry holds the -/// `Project` it was built from, and through it the whole `Solution`, and `latest` is never trimmed, so an -/// entry has to go when its project leaves the workspace. +/// Every snapshot roots the `Project`, and through it the `Solution`, it was built from: the entries follow the workspace. [] module internal ProjectSnapshotCache = let snapshots = AsyncMemoize(1000, 500, "SnapshotCache") - /// What the last snapshot of each project was built from, for the next one to reuse. let latest = ConcurrentDictionary * FSharpProjectOptions * FSharpProjectSnapshot>() let mutable private removals = 0 - /// How many removals the workspace has reported so far. A request that finds it moved while it was under - /// way may have built a snapshot of a project the workspace dropped, and the project's id cannot tell: - /// a reload brings the same ids back. let removalsSoFar () = System.Threading.Volatile.Read &removals @@ -195,6 +189,10 @@ module internal ProjectSnapshotCache = snapshots.Clear() latest.Clear() + let isGone (project: Project) removalsAtStart = + removalsSoFar () <> removalsAtStart + || not (project.Solution.Workspace.CurrentSolution.ContainsProject project.Id) + [] module private CheckerExtensions = @@ -254,7 +252,7 @@ module private CheckerExtensions = }) |> Seq.toList - let createProjectSnapshot (snapshotAccumulatorOpt) (project: Project) (options: FSharpProjectOptions option) = + let createProjectSnapshot removalsAtStart (snapshotAccumulatorOpt) (project: Project) (options: FSharpProjectOptions option) = cancellableTask { let! options = @@ -392,8 +390,9 @@ module private CheckerExtensions = let latestSnapshotData = project, projectVersion, referenceVersions, options, newSnapshot - ProjectSnapshotCache.latest.AddOrUpdate(project.Id, latestSnapshotData, (fun _ _ -> latestSnapshotData)) - |> ignore + if not (ProjectSnapshotCache.isGone project removalsAtStart) then + ProjectSnapshotCache.latest.AddOrUpdate(project.Id, latestSnapshotData, (fun _ _ -> latestSnapshotData)) + |> ignore return newSnapshot } @@ -410,25 +409,21 @@ module private CheckerExtensions = async { let removalsAtStart = ProjectSnapshotCache.removalsSoFar () - let! snapshot = - ProjectSnapshotCache.snapshots.Get( - key, - async { - let! ct = Async.CancellationToken - - return! - createProjectSnapshot snapshotAccumulatorOpt project options ct - |> Async.AwaitTask - } - ) - - // The handler that follows the workspace may have run before this request stored its entries, and a - // reload brings the project's id back, so the id says nothing: if a removal happened meanwhile, this - // project's entries go, and the next request builds them again. - if ProjectSnapshotCache.removalsSoFar () <> removalsAtStart then - ProjectSnapshotCache.forget project.Id - - return snapshot + try + return! + ProjectSnapshotCache.snapshots.Get( + key, + async { + let! ct = Async.CancellationToken + + return! + createProjectSnapshot removalsAtStart snapshotAccumulatorOpt project options ct + |> Async.AwaitTask + } + ) + finally + if ProjectSnapshotCache.isGone project removalsAtStart then + ProjectSnapshotCache.forget project.Id } let getProjectSnapshotForDocument (document: Document, options: FSharpProjectOptions) = diff --git a/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs b/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs index 77dc08672e2..1febfe81458 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/ProjectSnapshotCacheTests.fs @@ -1,8 +1,5 @@ // Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. -/// The snapshot caches hold the Roslyn project each snapshot was built from, so they have to follow the -/// workspace: a project that leaves it takes its entries along, also when it leaves while a snapshot of it is -/// being built. module FSharp.Editor.Tests.ProjectSnapshotCacheTests open System @@ -15,30 +12,24 @@ open Microsoft.VisualStudio.FSharp.Editor open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks open FSharp.Editor.Tests.Helpers -/// Text that arrives only when the test lets it, so a snapshot request that needs it stays pending, and that -/// says when a request has got as far as asking for it. type private GatedTextLoader(text: string, release: Task) = inherit TextLoader() let asked = TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) - let loaded () = - TextAndVersion.Create(SourceText.From text, VersionStamp.Create()) - member _.Asked = asked.Task override _.LoadTextAndVersionAsync(_options: LoadTextOptions, _cancellationToken: CancellationToken) : Task = task { asked.TrySetResult() |> ignore do! release - return loaded () + return TextAndVersion.Create(SourceText.From text, VersionStamp.Create()) } let private createProject () = RoslynTestHelpers.CreateSolution("module Test").Projects |> Seq.exactlyOne -/// A project whose only document gets its text when `release` completes. let private createProjectWithGatedText (release: Task) = let projectId = ProjectId.CreateNewId() let loader = GatedTextLoader("module Test", release) @@ -57,85 +48,80 @@ let private snapshotOf (project: Project) = project.GetFSharpProjectSnapshot() |> CancellableTask.start CancellationToken.None -/// The workspace raises its change events on a queue of its own: waits until the caches have heard of a removal. -let private waitForRemovalAfter (removalsBefore: int) = - Assert.True(SpinWait.SpinUntil((fun () -> ProjectSnapshotCache.removalsSoFar () <> removalsBefore), TimeSpan.FromSeconds 60.)) +let private waitForRemovalsAfter (removalsBefore: int) (projectId: ProjectId) = + let removed () = + ProjectSnapshotCache.removalsSoFar () <> removalsBefore + && not (ProjectSnapshotCache.latest.ContainsKey projectId) + + Assert.True(SpinWait.SpinUntil(removed, TimeSpan.FromSeconds 60.)) let private removeFromWorkspace (project: Project) = let workspace = project.Solution.Workspace - let before = ProjectSnapshotCache.removalsSoFar () + let removalsBefore = ProjectSnapshotCache.removalsSoFar () Assert.True(workspace.TryApplyChanges(workspace.CurrentSolution.RemoveProject project.Id)) - waitForRemovalAfter before + waitForRemovalsAfter removalsBefore project.Id -[] -let ``A project's snapshot is forgotten when the project leaves the workspace`` () : Task = - task { - let project = createProject () +let private reloadWorkspace (workspace: AdhocWorkspace) (projectInfo: ProjectInfo) = + let removalsBefore = ProjectSnapshotCache.removalsSoFar () + workspace.ClearSolution() - let! first = snapshotOf project - let! again = snapshotOf project - Assert.Same(first, again) - Assert.True(ProjectSnapshotCache.latest.ContainsKey project.Id) + workspace.AddSolution(SolutionInfo.Create(SolutionId.CreateNewId(), VersionStamp.Create(), "test.sln", [ projectInfo ])) + |> ignore - removeFromWorkspace project - // The handler counts the removal first and forgets after: wait for the entries to be gone. - Assert.True(SpinWait.SpinUntil((fun () -> not (ProjectSnapshotCache.latest.ContainsKey project.Id)), TimeSpan.FromSeconds 60.)) - - // Asked again, the same Roslyn project gets a snapshot built anew: the one it had is gone. - let! afterRemoval = snapshotOf project - Assert.NotSame(first, afterRemoval) - } + waitForRemovalsAfter removalsBefore projectInfo.Id -[] -let ``A snapshot being built when its project leaves the workspace is not kept`` () : Task = +/// Runs `disturb` while the project's first snapshot request waits for the text of its document. +let private snapshotBuiltWhile (disturb: Project -> ProjectInfo -> unit) = task { let release = TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) - let loader, _, project = createProjectWithGatedText release.Task - - // On a pool thread: a request starts on its caller's, which has to be free to let the text go. + let loader, projectInfo, project = createProjectWithGatedText release.Task let pending = Task.Run(Func>(fun () -> snapshotOf project)) do! loader.Asked - removeFromWorkspace project + disturb project projectInfo release.SetResult() - let! late = pending + let! built = pending + return project, box built + } + +let private assertForgotten (project: Project) (previous: obj) = + task { Assert.False(ProjectSnapshotCache.latest.ContainsKey project.Id) - // Nothing memoized either: the same Roslyn project gets a snapshot built anew. - let! later = snapshotOf project - Assert.NotSame(late, later) + let! rebuilt = snapshotOf project + Assert.NotSame(previous, rebuilt) } [] -let ``A snapshot being built when the solution is reloaded is not kept`` () : Task = +let ``A project's snapshots are forgotten when it leaves the workspace, and a later request keeps none`` () : Task = task { - let release = - TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) - - let loader, projectInfo, project = createProjectWithGatedText release.Task - let workspace = project.Solution.Workspace :?> AdhocWorkspace - - let pending = Task.Run(Func>(fun () -> snapshotOf project)) - do! loader.Asked + let project = createProject () - // A reload brings the same project id back, so a request that asked the workspace whether the id is - // still there would keep what it built for the project that went. - let before = ProjectSnapshotCache.removalsSoFar () - workspace.ClearSolution() + let! first = snapshotOf project + let! again = snapshotOf project + Assert.Same(first, again) - workspace.AddSolution(SolutionInfo.Create(SolutionId.CreateNewId(), VersionStamp.Create(), "test.sln", [ projectInfo ])) - |> ignore + removeFromWorkspace project + do! assertForgotten project first + Assert.False(ProjectSnapshotCache.latest.ContainsKey project.Id) + } - waitForRemovalAfter before - Assert.True(workspace.CurrentSolution.ContainsProject project.Id) +[] +let ``A snapshot being built when its project leaves the workspace is not kept`` () : Task = + task { + let! project, built = snapshotBuiltWhile (fun project _ -> removeFromWorkspace project) + do! assertForgotten project built + } - release.SetResult() - let! late = pending - Assert.False(ProjectSnapshotCache.latest.ContainsKey project.Id) +[] +let ``A snapshot being built when the solution is reloaded under the same project ids is not kept`` () : Task = + task { + let! project, built = + snapshotBuiltWhile (fun project projectInfo -> reloadWorkspace (project.Solution.Workspace :?> AdhocWorkspace) projectInfo) - let! later = snapshotOf project - Assert.NotSame(late, later) + Assert.True(project.Solution.Workspace.CurrentSolution.ContainsProject project.Id) + do! assertForgotten project built }