Fix contention in Roslyn language service

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1881089 Roslyn recently introduced async API for obtaining and releasing "batch" objects for applying updates in dotnet/roslyn#72424. This change increases the package versions, uses the newer API, fixes some obsolete usages, and gets things building by adding a few package references in order to break the tie on some assembly version conflicts during build. This is another iteration of #9455, which was reverted due to issues it created during signing. Unlike that prior PR, this does not remove Microsoft.VisualStudio.Internal.MicroBuild.Swix`, so I expct the same issues about duplicated items to appear.
dotnet · May 20, 2024 · c2beee0 · c2beee0
1 parent a8a739f
commit c2beee0
Show file tree

Hide file tree

Showing 17 changed files with 147 additions and 88 deletions.
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -7,6 +7,11 @@
     <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
   </PropertyGroup>
 
+  <!--
+      NOTE packages and their versions listed here must be specified in the dotnet-project-system-public-packages feed here:
+      https://dev.azure.com/azure-public/vside/_artifacts/feed/dotnet-project-system-public-packages
+  -->
+
   <ItemGroup>
     <!-- Infrastructure -->
     <!-- This package is deprecated. CodecovUploader is now the recommended package. -->
@@ -30,6 +35,7 @@
     <PackageVersion Include="Nerdbank.GitVersioning"                                                 Version="3.6.79-alpha" />
     <PackageVersion Include="Nerdbank.Streams"                                                       Version="2.11.72" />
     <PackageVersion Include="System.IO.Pipelines"                                                    Version="8.0.0" />
+    <PackageVersion Include="StreamJsonRpc"                                                          Version="2.18.53" />
 
     <!-- VS SDK -->
     <!-- https://dev.azure.com/azure-public/vside/_artifacts/feed/vssdk -->
@@ -77,18 +83,18 @@
     <PackageVersion Include="Microsoft.VisualStudio.ProjectSystem.XamlTypes"                         Version="$(CPSPackageVersion)" />
 
     <!-- Roslyn -->
-    <PackageVersion Include="Microsoft.CodeAnalysis"                                                 Version="4.8.0-3.final" />
-    <PackageVersion Include="Microsoft.CodeAnalysis.Common"                                          Version="4.8.0-3.final" />
+    <PackageVersion Include="Microsoft.CodeAnalysis"                                                 Version="4.11.0-1.24225.10" />
+    <PackageVersion Include="Microsoft.CodeAnalysis.Common"                                          Version="4.11.0-1.24225.10" />
     <PackageVersion Include="Microsoft.CSharp"                                                       Version="4.7.0" />
-    <PackageVersion Include="Microsoft.Net.Compilers.Toolset"                                        Version="4.10.0-3.24157.9" />
+    <PackageVersion Include="Microsoft.Net.Compilers.Toolset"                                        Version="4.11.0-1.24225.10" />
     <PackageVersion Include="Microsoft.VisualStudio.IntegrationTest.Utilities"                       Version="2.6.0-beta1-62113-02" />
-    <PackageVersion Include="Microsoft.VisualStudio.LanguageServices"                                Version="4.8.0-3.final" />
+    <PackageVersion Include="Microsoft.VisualStudio.LanguageServices"                                Version="4.11.0-1.24225.10" />
 
     <!-- Analyzers -->
     <PackageVersion Include="CSharpIsNullAnalyzer"                                                   Version="0.1.329" />
     <PackageVersion Include="Microsoft.CodeAnalysis.Analyzers"                                       Version="3.11.0-beta1.23472.1" />
-    <PackageVersion Include="Microsoft.CodeAnalysis.CSharp.CodeStyle"                                Version="4.8.0-3.final" />
-    <PackageVersion Include="Microsoft.CodeAnalysis.VisualBasic.CodeStyle"                           Version="4.8.0-3.final" />
+    <PackageVersion Include="Microsoft.CodeAnalysis.CSharp.CodeStyle"                                Version="4.11.0-1.24225.10" />
+    <PackageVersion Include="Microsoft.CodeAnalysis.VisualBasic.CodeStyle"                           Version="4.11.0-1.24225.10" />
     <PackageVersion Include="Roslyn.Diagnostics.Analyzers"                                           Version="3.11.0-beta1.23472.1" />
 
     <!-- NuGet -->

diff --git a/eng/imports/HostAgnostic.props b/eng/imports/HostAgnostic.props
@@ -58,6 +58,9 @@
 
     <!-- Host-Agnostic Visual Studio -->
     <PackageReference Include="Microsoft.VisualStudio.ImageCatalog" />
+
+    <!-- To resolve assembly versioning issue by pinning a specific version. Should remove in future when packages fixed. -->
+    <PackageReference Include="StreamJsonRpc" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(IsUnitTestProject)' == 'true'">

diff --git a/spelling.dic b/spelling.dic
@@ -71,6 +71,7 @@ unescape
 unescapes
 ushort
 usings
+vbproj
 vswhidbey
 watson
 xaml

diff --git a/src/Microsoft.VisualStudio.AppDesigner/Microsoft.VisualStudio.AppDesigner.vbproj b/src/Microsoft.VisualStudio.AppDesigner/Microsoft.VisualStudio.AppDesigner.vbproj
@@ -21,6 +21,7 @@
   <ItemGroup>
     <PackageReference Remove="Microsoft.VisualStudio.ProjectSystem*" />
   </ItemGroup>
+
   <ItemGroup>
     <Compile Include="..\Common\ManagedCodeMarkers.vb">
       <Link>ManagedCodeMarkers.vb</Link>

diff --git a/...ystem.Managed.VS/ProjectSystem/VS/LanguageServices/Handlers/AdditionalFilesItemHandler.cs b/...ystem.Managed.VS/ProjectSystem/VS/LanguageServices/Handlers/AdditionalFilesItemHandler.cs
@@ -48,7 +48,7 @@ private void AddToContextIfNotPresent(IWorkspaceProjectContext context, string f
             if (!_paths.Contains(fullPath))
             {
                 logger.WriteLine("Adding additional file '{0}'", fullPath);
-                context.AddAdditionalFile(fullPath, isActiveContext);
+                context.AddAdditionalFile(fullPath, folderNames: [], isActiveContext);
                 bool added = _paths.Add(fullPath);
                 Assumes.True(added);
             }

diff --git a/...m.Managed.VS/ProjectSystem/VS/LanguageServices/Handlers/CommandLineNotificationHandler.cs b/...m.Managed.VS/ProjectSystem/VS/LanguageServices/Handlers/CommandLineNotificationHandler.cs
@@ -18,18 +18,18 @@ internal class CommandLineNotificationHandler : IWorkspaceUpdateHandler, IComman
         public CommandLineNotificationHandler(UnconfiguredProject project)
         {
             // See FSharpCommandLineParserService.HandleCommandLineNotifications for an example of this export
-            CommandLineNotifications = new OrderPrecedenceImportCollection<Action<string, BuildOptions, BuildOptions>>(projectCapabilityCheckProvider: project);
+            CommandLineNotifications = new OrderPrecedenceImportCollection<Action<string?, BuildOptions, BuildOptions>>(projectCapabilityCheckProvider: project);
         }
 
         /// <remarks>
         /// See <see cref="FSharpCommandLineParserService.HandleCommandLineNotifications"/> for an export.
         /// </remarks>
         [ImportMany]
-        public OrderPrecedenceImportCollection<Action<string, BuildOptions, BuildOptions>> CommandLineNotifications { get; }
+        public OrderPrecedenceImportCollection<Action<string?, BuildOptions, BuildOptions>> CommandLineNotifications { get; }
 
         public void Handle(IWorkspaceProjectContext context, IComparable version, BuildOptions added, BuildOptions removed, ContextState state, IManagedProjectDiagnosticOutputService logger)
         {
-            foreach (Lazy<Action<string, BuildOptions, BuildOptions>, IOrderPrecedenceMetadataView> value in CommandLineNotifications)
+            foreach (Lazy<Action<string?, BuildOptions, BuildOptions>, IOrderPrecedenceMetadataView> value in CommandLineNotifications)
             {
                 value.Value(context.BinOutputPath, added, removed);
             }

diff --git a/...soft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/Workspace.cs b/...soft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/Workspace.cs
@@ -525,16 +525,14 @@ async Task ApplyInBatchAsync()
                     isActiveEditorContext: _activeEditorContextTracker.IsActiveEditorContext(ContextId),
                     isActiveConfiguration: IsPrimary);
 
-                Context.StartBatch();
-
                 try
                 {
+                    await using IAsyncDisposable _ = await Context.CreateBatchScopeAsync(cancellationToken);
+
                     applyFunc(update, contextState, cancellationToken);
                 }
                 finally
                 {
-                    await Context.EndBatchAsync();
-
                     UpdateProgressRegistration();
                 }
             }

diff --git a/...ft.VisualStudio.ProjectSystem.Managed/Microsoft.VisualStudio.ProjectSystem.Managed.csproj b/...ft.VisualStudio.ProjectSystem.Managed/Microsoft.VisualStudio.ProjectSystem.Managed.csproj
@@ -656,7 +656,7 @@
     <ItemGroup>
       <NpmContent Include="exports.json" />
       <!-- We have a runtime dependency on Microsoft.CodeAnalysis.dll in VS Code scenarios. -->
-      <NpmContent Include="$(PkgMicrosoft_CodeAnalysis_Common)\lib\net7.0\Microsoft.CodeAnalysis.dll" />
+      <NpmContent Include="$(PkgMicrosoft_CodeAnalysis_Common)\lib\net8.0\Microsoft.CodeAnalysis.dll" />
       <NpmContent Include="@(SatelliteDllsProjectOutputGroupOutput)">
         <PackagePath>%(SatelliteDllsProjectOutputGroupOutput.TargetPath)</PackagePath>
       </NpmContent>

diff --git a/...System.Managed.UnitTests/Mocks/IProjectThreadingServiceFactory.ProjectThreadingService.cs b/...System.Managed.UnitTests/Mocks/IProjectThreadingServiceFactory.ProjectThreadingService.cs
@@ -7,28 +7,13 @@ namespace Microsoft.VisualStudio.ProjectSystem
 {
     internal partial class IProjectThreadingServiceFactory
     {
-        private class ProjectThreadingService : IProjectThreadingService
+        private class ProjectThreadingService(bool verifyOnUIThread = true) : IProjectThreadingService
         {
-            private readonly bool _verifyOnUIThread;
-
-            public ProjectThreadingService(bool verifyOnUIThread = true) => _verifyOnUIThread = verifyOnUIThread;
-
-#pragma warning disable VSSDK005
             public JoinableTaskContextNode JoinableTaskContext { get; } = new JoinableTaskContextNode(new JoinableTaskContext());
-#pragma warning restore VSSDK005
 
             public JoinableTaskFactory JoinableTaskFactory => JoinableTaskContext.Factory;
 
-            public bool IsOnMainThread
-            {
-                get
-                {
-                    if (!_verifyOnUIThread)
-                        return true;
-
-                    return JoinableTaskContext.IsOnMainThread;
-                }
-            }
+            public bool IsOnMainThread => !verifyOnUIThread || JoinableTaskContext.IsOnMainThread;
 
             public void ExecuteSynchronously(Func<Task> asyncAction)
             {
@@ -42,16 +27,15 @@ public T ExecuteSynchronously<T>(Func<Task<T>> asyncAction)
 
             public void VerifyOnUIThread()
             {
-                if (!_verifyOnUIThread)
-                    return;
-
-                if (!IsOnMainThread)
+                if (verifyOnUIThread && !IsOnMainThread)
+                {
                     throw new InvalidOperationException();
+                }
             }
 
             public IDisposable SuppressProjectExecutionContext()
             {
-                return new DisposableObject();
+                return DisposableObject.Instance;
             }
 
             public void Fork(
@@ -68,6 +52,8 @@ public void Fork(
 
             private class DisposableObject : IDisposable
             {
+                public static IDisposable Instance { get; } = new DisposableObject();
+
                 public void Dispose()
                 {
                 }

diff --git a/...oft.VisualStudio.ProjectSystem.Managed.UnitTests/Mocks/IProjectThreadingServiceFactory.cs b/...oft.VisualStudio.ProjectSystem.Managed.UnitTests/Mocks/IProjectThreadingServiceFactory.cs
@@ -8,15 +8,5 @@ public static IProjectThreadingService Create(bool verifyOnUIThread = true)
         {
             return new ProjectThreadingService(verifyOnUIThread);
         }
-
-        public static IProjectThreadingService ImplementVerifyOnUIThread(Action action)
-        {
-            var mock = new Mock<IProjectThreadingService>();
-
-            mock.Setup(s => s.VerifyOnUIThread())
-                .Callback(action);
-
-            return mock.Object;
-        }
     }
 }
diff --git a/...tSystem.Managed.UnitTests/ProjectSystem/Properties/AssemblyInfoPropertiesProviderTests.cs b/...tSystem.Managed.UnitTests/ProjectSystem/Properties/AssemblyInfoPropertiesProviderTests.cs
@@ -183,6 +183,8 @@ public async Task ProjectFileProperties_GetEvaluatedPropertyAsync(string code, s
                     """[assembly: System.AssemblyDescriptionAttribute("MyDescription")]""")]
         public async Task SourceFileProperties_SetPropertyValueAsync(string code, string propertyName, string propertyValue, string expectedCode)
         {
+            using var _ = SynchronizationContextUtil.Suppress();
+
             var provider = CreateProviderForSourceFileValidation(code, out Workspace workspace);
             var projectFilePath = workspace.CurrentSolution.Projects.First().FilePath;
             Assumes.NotNull(projectFilePath);

diff --git a/...soft.VisualStudio.ProjectSystem.Managed.UnitTests/Utilities/SynchronizationContextUtil.cs b/...soft.VisualStudio.ProjectSystem.Managed.UnitTests/Utilities/SynchronizationContextUtil.cs
@@ -0,0 +1,46 @@
+// Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information.
+
+namespace Microsoft.VisualStudio;
+
+internal static class SynchronizationContextUtil
+{
+    /// <summary>
+    /// Sets a <see langword="null"/> <see cref="SynchronizationContext"/>, and restores
+    /// whatever context was <see cref="SynchronizationContext.Current"/> when disposed.
+    /// </summary>
+    /// <remarks>
+    /// This method is intended for use in tests where we use JTF and test code that
+    /// wants to switch to the main thread. Some of our tests don't actually construct
+    /// JTF in a way that creates a main thread that can be switched to. For these cases,
+    /// whatever thread creates the <see cref="Threading.JoinableTaskContext"/> is considered
+    /// the main thread. In unit tests, it's likely this is a thread pool thread, and
+    /// attempting to switch to the main thread will land on another thread pool thread.
+    /// <see cref="Threading.JoinableTaskFactory.SwitchToMainThreadAsync(CancellationToken)"/>
+    /// attempts to validate the switch was successful, and throws when not. This causes test
+    /// failures where we don't have a main thread to switch to. A workaround for this is
+    /// to suppress the synchronization context, which disables the check and allows the test
+    /// to pass.
+    /// </remarks>
+    /// <returns>An object that restores the previous synchronization context when disposed.</returns>
+    public static IDisposable Suppress()
+    {
+        SynchronizationContext old = SynchronizationContext.Current;
+
+        SynchronizationContext.SetSynchronizationContext(null);
+
+        return new SuppressionReleaser(old);
+    }
+
+    private sealed class SuppressionReleaser(SynchronizationContext old) : IDisposable
+    {
+        private int _isDisposed;
+
+        public void Dispose()
+        {
+            if (Interlocked.Exchange(ref _isDisposed, 1) is 0)
+            {
+                SynchronizationContext.SetSynchronizationContext(old);
+            }
+        }
+    }
+}
diff --git a/...oft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/Mocks/IWorkspaceProjectContextMock.cs b/...oft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/Mocks/IWorkspaceProjectContextMock.cs
@@ -7,6 +7,13 @@ internal class IWorkspaceProjectContextMock : AbstractMock<IWorkspaceProjectCont
         public IWorkspaceProjectContextMock()
         {
             SetupAllProperties();
+
+            Mock<IAsyncDisposable> batchScopeDisposable = new();
+
+            batchScopeDisposable.Setup(o => o.DisposeAsync());
+
+            Setup(o => o.CreateBatchScopeAsync(It.IsAny<CancellationToken>()))
+                .Returns(new ValueTask<IAsyncDisposable>(batchScopeDisposable.Object));
         }
     }
 }
diff --git a/...io.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/LanguageServices/WorkspaceTests.cs b/...io.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/LanguageServices/WorkspaceTests.cs
@@ -17,11 +17,11 @@ namespace Microsoft.VisualStudio.ProjectSystem.LanguageServices;
 public class WorkspaceTests
 {
     private static BuildOptions EmptyBuildOptions { get; } = new BuildOptions(
-            sourceFiles: ImmutableArray<CommandLineSourceFile>.Empty,
-            additionalFiles: ImmutableArray<CommandLineSourceFile>.Empty,
-            metadataReferences: ImmutableArray<CommandLineReference>.Empty,
-            analyzerReferences: ImmutableArray<CommandLineAnalyzerReference>.Empty,
-            analyzerConfigFiles: ImmutableArray<string>.Empty);
+        sourceFiles: [],
+        additionalFiles: [],
+        metadataReferences: [],
+        analyzerReferences: [],
+        analyzerConfigFiles: []);
 
     [Fact]
     public async Task Dispose_DisposesChainedDisposables()
@@ -366,7 +366,7 @@ public async Task EvaluationUpdate_InvokesEvaluationHandlersWhenChangesExist(boo
             }
             """);
 
-        Mock<IWorkspaceProjectContext> workspaceProjectContext = new(MockBehavior.Loose);
+        Mock<IWorkspaceProjectContext> workspaceProjectContext = new IWorkspaceProjectContextMock();
         Mock<IWorkspaceUpdateHandler> updateHandler = new(MockBehavior.Strict);
         Mock<IProjectEvaluationHandler> projectEvaluationHandler = updateHandler.As<IProjectEvaluationHandler>();
 
@@ -426,7 +426,7 @@ public async Task EvaluationUpdate_InvokesProjectEvaluationHandlersWhenChangesEx
             }
             """);
 
-        Mock<IWorkspaceProjectContext> workspaceProjectContext = new(MockBehavior.Loose);
+        Mock<IWorkspaceProjectContext> workspaceProjectContext = new IWorkspaceProjectContextMock();
         Mock<IWorkspaceUpdateHandler> updateHandler = new(MockBehavior.Strict);
         Mock<IProjectEvaluationHandler> projectEvaluationHandler = updateHandler.As<IProjectEvaluationHandler>();
 
@@ -474,7 +474,7 @@ public async Task EvaluationUpdate_InvokesSourceItemHandlersWhenChangesExist(boo
             }
             """);
 
-        Mock<IWorkspaceProjectContext> workspaceProjectContext = new(MockBehavior.Loose);
+        Mock<IWorkspaceProjectContext> workspaceProjectContext = new IWorkspaceProjectContextMock();
         Mock<IWorkspaceUpdateHandler> updateHandler = new(MockBehavior.Strict);
         Mock<ISourceItemsHandler> sourceItemsHandler = updateHandler.As<ISourceItemsHandler>();
 
@@ -534,7 +534,7 @@ public async Task BuildUpdate_InvokesCommandLineHandlerWhenChangesExist(bool any
               }
               """);
 
-        Mock<IWorkspaceProjectContext> workspaceProjectContext = new(MockBehavior.Loose);
+        Mock<IWorkspaceProjectContext> workspaceProjectContext = new IWorkspaceProjectContextMock();
         Mock<IWorkspaceUpdateHandler> updateHandler = new(MockBehavior.Strict);
         Mock<ICommandLineHandler> commandLineHandler = updateHandler.As<ICommandLineHandler>();
 
@@ -622,14 +622,14 @@ await ApplyBuildAsync(
     }
 
     [Fact]
-    public async Task Update_StartBatchThrows()
+    public async Task Update_CreateBatchScopeAsyncThrows()
     {
         Exception ex = new("Error starting batch");
 
         Mock<IWorkspaceProjectContext> workspaceProjectContext = new(MockBehavior.Strict);
 
-        // StartBatch throws
-        workspaceProjectContext.Setup(o => o.StartBatch()).Throws(ex);
+        // CreateBatchScopeAsync throws
+        workspaceProjectContext.Setup(o => o.CreateBatchScopeAsync(It.IsAny<CancellationToken>())).Throws(ex);
 
         // Expect disposal
         workspaceProjectContext.Setup(o => o.Dispose());
@@ -640,15 +640,17 @@ public async Task Update_StartBatchThrows()
     }
 
     [Fact]
-    public async Task Update_EndBatchAsyncThrows()
+    public async Task Update_BatchScopeDisposalThrows()
     {
         Exception ex = new("Error starting batch");
 
         Mock<IWorkspaceProjectContext> workspaceProjectContext = new(MockBehavior.Strict);
+        Mock<IAsyncDisposable> batchScopeDisposable = new();
 
-        // EndBatchAsync throws
-        workspaceProjectContext.Setup(o => o.StartBatch());
-        workspaceProjectContext.Setup(o => o.EndBatchAsync()).Throws(ex);
+        // DisposeAsync throws
+        batchScopeDisposable.Setup(o => o.DisposeAsync()).Throws(ex);
+
+        workspaceProjectContext.Setup(o => o.CreateBatchScopeAsync(It.IsAny<CancellationToken>())).Returns(new ValueTask<IAsyncDisposable>(batchScopeDisposable.Object));
 
         // Expect disposal
         workspaceProjectContext.Setup(o => o.Dispose());
@@ -817,7 +819,7 @@ private static async Task<Workspace> CreateInstanceAsync(
         activeWorkspaceProjectContextTracker ??= IActiveEditorContextTrackerFactory.Create();
         commandLineParserServices ??= new(ImportOrderPrecedenceComparer.PreferenceOrder.PreferredComesFirst) { commandLineParserService.Object };
         dataProgressTrackerService ??= IDataProgressTrackerServiceFactory.Create();
-        workspaceProjectContext ??= Mock.Of<IWorkspaceProjectContext>(MockBehavior.Loose);
+        workspaceProjectContext ??= new IWorkspaceProjectContextMock().Object;
         workspaceProjectContextFactory ??= IWorkspaceProjectContextFactoryFactory.ImplementCreateProjectContext(delegate { return workspaceProjectContext; });
         faultHandlerService ??= IProjectFaultHandlerServiceFactory.Create();
 #pragma warning disable VSSDK005
-Original file line number
+Diff line change
@@ Expand Up / @@ -71,6 +71,7 @@ unescape @@
     unescapes
     ushort
     usings
+    vbproj
     vswhidbey
     watson
     xaml
@@ Expand Down @@