Merge pull request #16943 from heejaechang/fixRace2

changed all caller to call JsonRpc.StartListener once all states are set
dotnet · Feb 4, 2017 · 53f2a87 · 53f2a87
2 parents 0797ac4 + 524b609
commit 53f2a87
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 9 deletions.
diff --git a/src/EditorFeatures/TestUtilities/Remote/InProcRemostHostClient.cs b/src/EditorFeatures/TestUtilities/Remote/InProcRemostHostClient.cs
@@ -16,7 +16,7 @@
 
 namespace Roslyn.Test.Utilities.Remote
 {
-    internal class InProcRemoteHostClient : RemoteHostClient
+    internal sealed class InProcRemoteHostClient : RemoteHostClient
     {
         private readonly InProcRemoteServices _inprocServices;
         private readonly JsonRpc _rpc;
@@ -47,10 +47,12 @@ private InProcRemoteHostClient(Workspace workspace, InProcRemoteServices inprocS
         {
             _inprocServices = inprocServices;
 
-            _rpc = JsonRpc.Attach(stream, target: this);
+            _rpc = new JsonRpc(stream, stream, target: this);
 
             // handle disconnected situation
             _rpc.Disconnected += OnRpcDisconnected;
+
+            _rpc.StartListening();
         }
 
         public AssetStorage AssetStorage => _inprocServices.AssetStorage;

diff --git a/src/VisualStudio/Core/Next/Remote/JsonRpcClient.cs b/src/VisualStudio/Core/Next/Remote/JsonRpcClient.cs
@@ -25,7 +25,8 @@ public JsonRpcClient(
             var target = useThisAsCallback ? this : callbackTarget;
             _cancellationToken = cancellationToken;
 
-            _rpc = JsonRpc.Attach(stream, target);
+            _rpc = new JsonRpc(stream, stream, target);
+
             _rpc.Disconnected += OnDisconnected;
         }
 
@@ -84,6 +85,13 @@ public void Dispose()
             _rpc.Dispose();
         }
 
+        protected void StartListening()
+        {
+            // due to this issue - https://github.com/dotnet/roslyn/issues/16900#issuecomment-277378950
+            // _rpc need to be explicitly started
+            _rpc.StartListening();
+        }
+
         protected virtual void OnDisposed()
         {
             // do nothing

diff --git a/src/VisualStudio/Core/Next/Remote/JsonRpcSession.cs b/src/VisualStudio/Core/Next/Remote/JsonRpcSession.cs
@@ -114,6 +114,8 @@ public ServiceJsonRpcClient(Stream stream, object callbackTarget, CancellationTo
             {
                 // this one doesn't need cancellation token since it has nothing to cancel
                 _callbackTarget = callbackTarget;
+
+                StartListening();
             }
         }
 
@@ -137,6 +139,8 @@ public SnapshotJsonRpcClient(JsonRpcSession owner, Stream stream, CancellationTo
             {
                 _owner = owner;
                 _source = new CancellationTokenSource();
+
+                StartListening();
             }
 
             private PinnedRemotableDataScope PinnedScope => _owner.PinnedScope;

diff --git a/src/VisualStudio/Core/Next/Remote/ServiceHubRemoteHostClient.cs b/src/VisualStudio/Core/Next/Remote/ServiceHubRemoteHostClient.cs
@@ -18,7 +18,7 @@ namespace Microsoft.VisualStudio.LanguageServices.Remote
     using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
     using Workspace = Microsoft.CodeAnalysis.Workspace;
 
-    internal partial class ServiceHubRemoteHostClient : RemoteHostClient
+    internal sealed partial class ServiceHubRemoteHostClient : RemoteHostClient
     {
         private readonly HubClient _hubClient;
         private readonly JsonRpc _rpc;
@@ -81,10 +81,12 @@ private ServiceHubRemoteHostClient(
             _hubClient = hubClient;
             _hostGroup = hostGroup;
 
-            _rpc = JsonRpc.Attach(stream, target: this);
+            _rpc = new JsonRpc(stream, stream, target: this);
 
             // handle disconnected situation
             _rpc.Disconnected += OnRpcDisconnected;
+
+            _rpc.StartListening();
         }
 
         protected override async Task<Session> CreateServiceSessionAsync(string serviceName, PinnedRemotableDataScope snapshot, object callbackTarget, CancellationToken cancellationToken)

diff --git a/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService.cs b/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService.cs
@@ -9,8 +9,9 @@ namespace Microsoft.CodeAnalysis.Remote
     internal partial class CodeAnalysisService : ServiceHubServiceBase
     {
         public CodeAnalysisService(Stream stream, IServiceProvider serviceProvider) :
-            base(stream, serviceProvider)
+            base(serviceProvider, stream)
         {
+            Rpc.StartListening();
         }
     }
 }
diff --git a/src/Workspaces/Remote/ServiceHub/Services/RemoteHostService.cs b/src/Workspaces/Remote/ServiceHub/Services/RemoteHostService.cs
@@ -39,9 +39,10 @@ static RemoteHostService()
         }
 
         public RemoteHostService(Stream stream, IServiceProvider serviceProvider) :
-            base(stream, serviceProvider)
+            base(serviceProvider, stream)
         {
             // this service provide a way for client to make sure remote host is alive
+            Rpc.StartListening();
         }
 
         public string Connect(string host)

diff --git a/src/Workspaces/Remote/ServiceHub/Services/RemoteSymbolSearchUpdateEngine.cs b/src/Workspaces/Remote/ServiceHub/Services/RemoteSymbolSearchUpdateEngine.cs
@@ -13,10 +13,12 @@ internal partial class RemoteSymbolSearchUpdateEngine : ServiceHubServiceBase, I
         private readonly SymbolSearchUpdateEngine _updateEngine;
 
         public RemoteSymbolSearchUpdateEngine(Stream stream, IServiceProvider serviceProvider)
-            : base(stream, serviceProvider)
+            : base(serviceProvider, stream)
         {
             _updateEngine = new SymbolSearchUpdateEngine(
                 new LogService(this), updateCancellationToken: this.CancellationToken);
+
+            Rpc.StartListening();
         }
 
         public Task UpdateContinuouslyAsync(string sourceName, string localSettingsDirectory)

diff --git a/src/Workspaces/Remote/ServiceHub/Services/SnapshotService.cs b/src/Workspaces/Remote/ServiceHub/Services/SnapshotService.cs
@@ -20,9 +20,11 @@ internal partial class SnapshotService : ServiceHubServiceBase
         private AssetSource _source;
 
         public SnapshotService(Stream stream, IServiceProvider serviceProvider) :
-            base(stream, serviceProvider)
+            base(serviceProvider, stream)
         {
             _gate = new object();
+
+            Rpc.StartListening();
         }
 
         public override void Initialize(int sessionId, byte[] solutionChecksum)

diff --git a/src/Workspaces/Remote/ServiceHub/Shared/ServiceHubServiceBase.cs b/src/Workspaces/Remote/ServiceHub/Shared/ServiceHubServiceBase.cs
@@ -29,6 +29,7 @@ internal abstract class ServiceHubServiceBase : IDisposable
         private Checksum _solutionChecksumOpt;
         private RoslynServices _lazyRoslynServices;
 
+        [Obsolete("For backward compatibility. this will be removed once all callers moved to new ctor")]
         protected ServiceHubServiceBase(Stream stream, IServiceProvider serviceProvider)
         {
             _instanceId = Interlocked.Add(ref s_instanceId, 1);
@@ -46,6 +47,26 @@ protected ServiceHubServiceBase(Stream stream, IServiceProvider serviceProvider)
             Rpc.Disconnected += OnRpcDisconnected;
         }
 
+        protected ServiceHubServiceBase(IServiceProvider serviceProvider, Stream stream)
+        {
+            _instanceId = Interlocked.Add(ref s_instanceId, 1);
+
+            // in unit test, service provider will return asset storage, otherwise, use the default one
+            AssetStorage = (AssetStorage)serviceProvider.GetService(typeof(AssetStorage)) ?? AssetStorage.Default;
+
+            Logger = (TraceSource)serviceProvider.GetService(typeof(TraceSource));
+            Logger.TraceInformation($"{DebugInstanceString} Service instance created");
+
+            _cancellationTokenSource = new CancellationTokenSource();
+            CancellationToken = _cancellationTokenSource.Token;
+
+            // due to this issue - https://github.com/dotnet/roslyn/issues/16900#issuecomment-277378950
+            // all sub type must explicitly start JsonRpc once everything is
+            // setup
+            Rpc = new JsonRpc(stream, stream, this);
+            Rpc.Disconnected += OnRpcDisconnected;
+        }
+
         protected string DebugInstanceString => $"{GetType()} ({_instanceId})";
 
         protected RoslynServices RoslynServices