From a55491a3f4ca20e0ab23d07dac5a95594436a9cd Mon Sep 17 00:00:00 2001 From: Leyla <54935347+bigfluffycookie@users.noreply.github.com> Date: Fri, 8 Sep 2023 14:11:34 +0200 Subject: [PATCH] Make SSESessionManager ctor free threaded (#4863) * Make SSESessionManager ctor free threaded * pr fixes --- .../SSESessionManagerTests.cs | 66 +++++++++++-------- src/ConnectedMode/ConnectedModePackage.cs | 21 ++++-- .../ServerSentEvents/SSESessionManager.cs | 21 +++--- 3 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/ConnectedMode.UnitTests/ServerSentEvents/SSESessionManagerTests.cs b/src/ConnectedMode.UnitTests/ServerSentEvents/SSESessionManagerTests.cs index 15bf99447e..65a0566465 100644 --- a/src/ConnectedMode.UnitTests/ServerSentEvents/SSESessionManagerTests.cs +++ b/src/ConnectedMode.UnitTests/ServerSentEvents/SSESessionManagerTests.cs @@ -39,12 +39,30 @@ public void MefCtor_CheckIsExported() activeSolutionBoundTrackerMock.SetupGet(tracker => tracker.CurrentConfiguration) .Returns(BindingConfiguration.Standalone); - MefTestHelpers.CheckTypeCanBeImported( + MefTestHelpers.CheckTypeCanBeImported( MefTestHelpers.CreateExport(activeSolutionBoundTrackerMock.Object), MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport()); } + [TestMethod] + public void Ctor_DoesNotCallAnyServices_BesidesExpected() + { + var activeSolutionBoundTracker = new Mock(); + var sseSessionFactory = new Mock(); + var logger = new Mock(); + + var _ = new SSESessionManager(activeSolutionBoundTracker.Object, sseSessionFactory.Object, logger.Object); + + // The MEF constructor should be free-threaded, which it will be if + // it doesn't make any external calls. + + activeSolutionBoundTracker.VerifyAdd(tracker => tracker.SolutionBindingChanged += It.IsAny>(), Times.Once); + activeSolutionBoundTracker.VerifyNoOtherCalls(); + sseSessionFactory.Invocations.Should().BeEmpty(); + logger.Invocations.Should().BeEmpty(); + } + [TestMethod] public void Ctor_SubscribesToBindingChangedEvent() { @@ -58,47 +76,34 @@ public void Ctor_SubscribesToBindingChangedEvent() } [TestMethod] - public void Ctor_ForceConnectsAfterSubscriptionToBindingChangedEvent() + public void CreateSessionIfInConnectedMode_WhenInStandaloneModeOnCreation_DoesNotCreateSession() { - var isSubscribed = false; - var testScope = new TestScope(TestScope.CreateConnectedModeBindingConfiguration(DefaultProjectKey)); - testScope.ActiveSolutionBoundTrackerMock - .SetupAdd(solutionTracker => solutionTracker.SolutionBindingChanged += It.IsAny>()). - Callback(() => - { - isSubscribed.Should().BeFalse(); - isSubscribed = true; - }); - testScope.SetUpSSEFactoryToReturnNoOpSSESession(DefaultProjectKey, factoryMockCallback: () => isSubscribed.Should().BeTrue()); + var bindingConfig = BindingConfiguration.Standalone; + var testScope = new TestScope(bindingConfig); var _ = testScope.CreateTestSubject(); + var testSubject = testScope.CreateTestSubject(); - testScope.ActiveSolutionBoundTrackerMock.VerifyAdd(tracker => tracker.SolutionBindingChanged += It.IsAny>(), Times.Once); - testScope.SSESessionFactoryMock.Verify(factory => factory.Create(DefaultProjectKey, It.IsAny()), Times.Once); + testSubject.CreateSessionIfInConnectedMode(bindingConfig); + + testScope.SSESessionFactoryMock.Verify(factory => factory.Create(DefaultProjectKey, It.IsAny()), Times.Never); } [TestMethod] - public void Ctor_WhenInConnectedModeOnCreation_CreatesSession() + public void CreateSessionIfInConnectedMode_WhenInConnectedModeOnCreation_CreatesSession() { - var testScope = new TestScope(TestScope.CreateConnectedModeBindingConfiguration(DefaultProjectKey)); + var bindingConfig = TestScope.CreateConnectedModeBindingConfiguration(DefaultProjectKey); + + var testScope = new TestScope(bindingConfig); var sessionMock = testScope.SetUpSSEFactoryToReturnNoOpSSESession(DefaultProjectKey); - var _ = testScope.CreateTestSubject(); + var testSubject = testScope.CreateTestSubject(); + testSubject.CreateSessionIfInConnectedMode(bindingConfig); testScope.SSESessionFactoryMock.Verify(factory => factory.Create(DefaultProjectKey, It.IsAny()), Times.Once); sessionMock.Verify(session => session.PumpAllAsync(), Times.Once); } - [TestMethod] - public void Ctor_WhenInStandaloneModeOnCreation_DoesNotCreateSession() - { - var testScope = new TestScope(BindingConfiguration.Standalone); - - var _ = testScope.CreateTestSubject(); - - testScope.SSESessionFactoryMock.Verify(factory => factory.Create(DefaultProjectKey, It.IsAny()), Times.Never); - } - [TestMethod] public void OnSolutionChanged_WhenChangesFromStandaloneToConnected_CreatesSessionAndLaunchesIt() { @@ -153,11 +158,14 @@ public void OnSolutionChanged_WhenChangesFromConnectedToConnected_CancelsSession [DataTestMethod] public async Task OnSessionFailed_CancelsSessionAndStartsNewOne() { - var testScope = new TestScope(TestScope.CreateConnectedModeBindingConfiguration(DefaultProjectKey)); + var bindingConfig = TestScope.CreateConnectedModeBindingConfiguration(DefaultProjectKey); + + var testScope = new TestScope(bindingConfig); var sessionMock1 = testScope.SetUpSSEFactoryToReturnNoOpSSESession(DefaultProjectKey); sessionMock1.Setup(session => session.Dispose()); - var _ = testScope.CreateTestSubject(); + var testSubject = testScope.CreateTestSubject(); + testSubject.CreateSessionIfInConnectedMode(bindingConfig); var sessionMock2 = testScope.SetUpSSEFactoryToReturnNoOpSSESession(DefaultProjectKey); diff --git a/src/ConnectedMode/ConnectedModePackage.cs b/src/ConnectedMode/ConnectedModePackage.cs index 6432dcf946..66e2b0a3d7 100644 --- a/src/ConnectedMode/ConnectedModePackage.cs +++ b/src/ConnectedMode/ConnectedModePackage.cs @@ -43,7 +43,7 @@ namespace SonarLint.VisualStudio.ConnectedMode [Guid("dd3427e0-7bb2-4a51-b00a-ddae2c32c7ef")] public sealed class ConnectedModePackage : AsyncPackage { - private ISSESessionManager sseSessionManager; + private SSESessionManager sseSessionManager; private IIssueServerEventsListener issueServerEventsListener; private IQualityProfileServerEventsListener qualityProfileServerEventsListener; private ServerSuppressionsChangedHandler serverSuppressionsChangedHandler; @@ -64,7 +64,7 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke logger.WriteLine(Resources.Package_Initializing); - sseSessionManager = componentModel.GetService(); + LoadServicesAndDoInitialUpdates(componentModel); issueServerEventsListener = componentModel.GetService(); issueServerEventsListener.ListenAsync().Forget(); @@ -84,17 +84,24 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke hotspotStoreMonitor = componentModel.GetService(); await hotspotStoreMonitor.InitializeAsync(); - // Trigger an initial update of suppressions (These classes might have missed the initial solution binding - // event from the ActiveSolutionBoundTracker) - // See https://github.com/SonarSource/sonarlint-visualstudio/issues/3886 + logger.WriteLine(Resources.Package_Initialized); + } + + /// + /// Trigger an initial update of classes that need them. (These classes might have missed the initial solution binding + /// event from the ActiveSolutionBoundTracker) + /// See https://github.com/SonarSource/sonarlint-visualstudio/issues/3886 + /// + private void LoadServicesAndDoInitialUpdates(IComponentModel componentModel) + { + sseSessionManager = componentModel.GetService(); + sseSessionManager.CreateSessionIfInConnectedMode(); importBeforeInstallTrigger = componentModel.GetService(); importBeforeInstallTrigger.TriggerUpdateAsync().Forget(); var updater = componentModel.GetService(); updater.UpdateAllServerSuppressionsAsync().Forget(); var hotspotsUpdater = componentModel.GetService(); hotspotsUpdater.UpdateAllServerHotspotsAsync().Forget(); - - logger.WriteLine(Resources.Package_Initialized); } protected override void Dispose(bool disposing) diff --git a/src/ConnectedMode/ServerSentEvents/SSESessionManager.cs b/src/ConnectedMode/ServerSentEvents/SSESessionManager.cs index 2ee6038313..f085325a9b 100644 --- a/src/ConnectedMode/ServerSentEvents/SSESessionManager.cs +++ b/src/ConnectedMode/ServerSentEvents/SSESessionManager.cs @@ -27,16 +27,9 @@ namespace SonarLint.VisualStudio.ConnectedMode.ServerSentEvents { - /// - /// Reacts to project changes and opens/closes Server Sent Events sessions - /// - internal interface ISSESessionManager : IDisposable - { - } - - [Export(typeof(ISSESessionManager))] + [Export(typeof(SSESessionManager))] [PartCreationPolicy(CreationPolicy.Shared)] - internal sealed class SSESessionManager : ISSESessionManager + internal sealed class SSESessionManager : IDisposable { private const int DelayTimeBetweenRetriesInMilliseconds = 1000; @@ -59,8 +52,6 @@ public SSESessionManager(IActiveSolutionBoundTracker activeSolutionBoundTracker, this.logger = logger; activeSolutionBoundTracker.SolutionBindingChanged += SolutionBindingChanged; - - CreateSessionIfInConnectedMode(activeSolutionBoundTracker.CurrentConfiguration); } public void Dispose() @@ -81,8 +72,14 @@ private void SolutionBindingChanged(object sender, ActiveSolutionBindingEventArg CreateSessionIfInConnectedMode(activeSolutionBindingEventArgs.Configuration); } - private void CreateSessionIfInConnectedMode(BindingConfiguration bindingConfiguration) + /// + /// Creates a new session if in connected mode. If no binding configuration is provided the + /// ActiveSolutionBoundTracker.CurrentConfiguration will be used. + /// + public void CreateSessionIfInConnectedMode(BindingConfiguration bindingConfiguration = null) { + if (bindingConfiguration == null) { bindingConfiguration = activeSolutionBoundTracker.CurrentConfiguration; } + lock (syncRoot) { EndCurrentSession();