Skip to content

Commit 24bcf13

Browse files
sharwellsharwell
committed
Remove ClientDisconnectedSource
This source is unnecessary since operations will fail anyway after a disconnection occurs.
1 parent c020fe8 commit 24bcf13

File tree

8 files changed

+26
-56
lines changed

8 files changed

+26
-56
lines changed

‎src/Workspaces/Remote/Core/RemoteCallback.cs‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@
99
using System.IO.Pipelines;
1010
using System.Threading;
1111
using System.Threading.Tasks;
12-
using MessagePack;
1312
using Microsoft.CodeAnalysis.ErrorReporting;
14-
using Nerdbank.Streams;
15-
using Newtonsoft.Json;
1613
using Roslyn.Utilities;
1714
using StreamJsonRpc;
1815

@@ -30,12 +27,9 @@ internal readonly struct RemoteCallback<T>
3027
{
3128
private readonly T _callback;
3229

33-
public readonly CancellationTokenSource ClientDisconnectedSource;
34-
35-
public RemoteCallback(T callback, CancellationTokenSource clientDisconnectedSource)
30+
public RemoteCallback(T callback)
3631
{
3732
_callback = callback;
38-
ClientDisconnectedSource = clientDisconnectedSource;
3933
}
4034

4135
public async ValueTask InvokeAsync(Func<T, CancellationToken, ValueTask> invocation, CancellationToken cancellationToken)
@@ -46,7 +40,7 @@ public async ValueTask InvokeAsync(Func<T, CancellationToken, ValueTask> invocat
4640
}
4741
catch (Exception exception) when (ReportUnexpectedException(exception, cancellationToken))
4842
{
49-
throw OnUnexpectedException(cancellationToken);
43+
throw OnUnexpectedException(exception, cancellationToken);
5044
}
5145
}
5246

@@ -58,7 +52,7 @@ public async ValueTask<TResult> InvokeAsync<TResult>(Func<T, CancellationToken,
5852
}
5953
catch (Exception exception) when (ReportUnexpectedException(exception, cancellationToken))
6054
{
61-
throw OnUnexpectedException(cancellationToken);
55+
throw OnUnexpectedException(exception, cancellationToken);
6256
}
6357
}
6458

@@ -76,7 +70,7 @@ public async ValueTask<TResult> InvokeAsync<TResult>(
7670
}
7771
catch (Exception exception) when (ReportUnexpectedException(exception, cancellationToken))
7872
{
79-
throw OnUnexpectedException(cancellationToken);
73+
throw OnUnexpectedException(exception, cancellationToken);
8074
}
8175
}
8276

@@ -87,7 +81,7 @@ public async ValueTask<TResult> InvokeAsync<TResult>(
8781
// 3) Remote exception - an exception was thrown by the callee
8882
// 4) Cancelation
8983
//
90-
private bool ReportUnexpectedException(Exception exception, CancellationToken cancellationToken)
84+
private static bool ReportUnexpectedException(Exception exception, CancellationToken cancellationToken)
9185
{
9286
if (exception is IOException)
9387
{
@@ -114,20 +108,24 @@ private bool ReportUnexpectedException(Exception exception, CancellationToken ca
114108
// as any observation of ConnectionLostException indicates a bug (e.g. https://github.com/microsoft/vs-streamjsonrpc/issues/549).
115109
if (exception is ConnectionLostException)
116110
{
117-
ClientDisconnectedSource.Cancel();
118-
119111
return true;
120112
}
121113

122114
// Indicates bug on client side or in serialization, report NFW and propagate the exception.
123115
return FatalError.ReportWithoutCrashAndPropagate(exception);
124116
}
125117

126-
private static Exception OnUnexpectedException(CancellationToken cancellationToken)
118+
private static Exception OnUnexpectedException(Exception exception, CancellationToken cancellationToken)
127119
{
128120
cancellationToken.ThrowIfCancellationRequested();
129121

130-
// If this is hit the cancellation token passed to the service implementation did not use the correct token.
122+
if (exception is ConnectionLostException)
123+
{
124+
throw new OperationCanceledException(exception.Message, exception);
125+
}
126+
127+
// If this is hit the cancellation token passed to the service implementation did not use the correct token,
128+
// and the resulting exception was not a ConnectionLostException.
131129
return ExceptionUtilities.Unreachable;
132130
}
133131
}

‎src/Workspaces/Remote/ServiceHub/Host/SolutionAssetSource.cs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,10 @@ namespace Microsoft.CodeAnalysis.Remote
1919
internal sealed class SolutionAssetSource : IAssetSource
2020
{
2121
private readonly ServiceBrokerClient _client;
22-
private readonly CancellationTokenSource _clientDisconnectedSource;
2322

24-
public SolutionAssetSource(ServiceBrokerClient client, CancellationTokenSource clientDisconnectedSource)
23+
public SolutionAssetSource(ServiceBrokerClient client)
2524
{
2625
_client = client;
27-
_clientDisconnectedSource = clientDisconnectedSource;
2826
}
2927

3028
public async ValueTask<ImmutableArray<(Checksum, object)>> GetAssetsAsync(int scopeId, ISet<Checksum> checksums, ISerializerService serializerService, CancellationToken cancellationToken)
@@ -35,7 +33,7 @@ public SolutionAssetSource(ServiceBrokerClient client, CancellationTokenSource c
3533
using var provider = await _client.GetProxyAsync<ISolutionAssetProvider>(SolutionAssetProvider.ServiceDescriptor, cancellationToken).ConfigureAwait(false);
3634
Contract.ThrowIfNull(provider.Proxy);
3735

38-
return await new RemoteCallback<ISolutionAssetProvider>(provider.Proxy, _clientDisconnectedSource).InvokeAsync(
36+
return await new RemoteCallback<ISolutionAssetProvider>(provider.Proxy).InvokeAsync(
3937
(proxy, pipeWriter, cancellationToken) => proxy.GetAssetsAsync(pipeWriter, scopeId, checksums.ToArray(), cancellationToken),
4038
(pipeReader, cancellationToken) => RemoteHostAssetSerialization.ReadDataAsync(pipeReader, scopeId, checksums, serializerService, cancellationToken),
4139
cancellationToken).ConfigureAwait(false);
@@ -49,7 +47,7 @@ public async ValueTask<bool> IsExperimentEnabledAsync(string experimentName, Can
4947
using var provider = await _client.GetProxyAsync<ISolutionAssetProvider>(SolutionAssetProvider.ServiceDescriptor, cancellationToken).ConfigureAwait(false);
5048
Contract.ThrowIfNull(provider.Proxy);
5149

52-
return await new RemoteCallback<ISolutionAssetProvider>(provider.Proxy, _clientDisconnectedSource).InvokeAsync(
50+
return await new RemoteCallback<ISolutionAssetProvider>(provider.Proxy).InvokeAsync(
5351
(self, cancellationToken) => provider.Proxy.IsExperimentEnabledAsync(experimentName, cancellationToken),
5452
cancellationToken).ConfigureAwait(false);
5553
}

‎src/Workspaces/Remote/ServiceHub/Services/BrokeredServiceBase.FactoryBase.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using System.Diagnostics;
99
using System.IO;
1010
using System.IO.Pipelines;
11-
using System.Threading;
1211
using System.Threading.Tasks;
1312
using Microsoft.ServiceHub.Framework;
1413
using Microsoft.ServiceHub.Framework.Services;
@@ -74,7 +73,7 @@ internal TService Create(
7473
var serviceHubTraceSource = (TraceSource)hostProvidedServices.GetService(typeof(TraceSource));
7574
var serverConnection = descriptor.WithTraceSource(serviceHubTraceSource).ConstructRpcConnection(pipe);
7675

77-
var args = new ServiceConstructionArguments(hostProvidedServices, serviceBroker, new CancellationTokenSource());
76+
var args = new ServiceConstructionArguments(hostProvidedServices, serviceBroker);
7877
var service = CreateService(args, descriptor, serverConnection, serviceActivationOptions.ClientRpcTarget);
7978

8079
serverConnection.AddLocalRpcTarget(service);
@@ -106,7 +105,7 @@ protected sealed override TService CreateService(
106105
{
107106
Contract.ThrowIfNull(descriptor.ClientInterface);
108107
var callback = (TCallback)(clientRpcTarget ?? serverConnection.ConstructRpcClient(descriptor.ClientInterface));
109-
return CreateService(arguments, new RemoteCallback<TCallback>(callback, arguments.ClientDisconnectedSource));
108+
return CreateService(arguments, new RemoteCallback<TCallback>(callback));
110109
}
111110
}
112111
}

‎src/Workspaces/Remote/ServiceHub/Services/BrokeredServiceBase.ServiceConstructionArguments.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
#nullable enable
66

77
using System;
8-
using System.Threading;
98
using Microsoft.ServiceHub.Framework;
109

1110
namespace Microsoft.CodeAnalysis.Remote
@@ -16,13 +15,11 @@ internal readonly struct ServiceConstructionArguments
1615
{
1716
public readonly IServiceProvider ServiceProvider;
1817
public readonly IServiceBroker ServiceBroker;
19-
public readonly CancellationTokenSource ClientDisconnectedSource;
2018

21-
public ServiceConstructionArguments(IServiceProvider serviceProvider, IServiceBroker serviceBroker, CancellationTokenSource clientDisconnectedSource)
19+
public ServiceConstructionArguments(IServiceProvider serviceProvider, IServiceBroker serviceBroker)
2220
{
2321
ServiceProvider = serviceProvider;
2422
ServiceBroker = serviceBroker;
25-
ClientDisconnectedSource = clientDisconnectedSource;
2623
}
2724
}
2825
}

‎src/Workspaces/Remote/ServiceHub/Services/BrokeredServiceBase.cs‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,10 @@
66

77
using System;
88
using System.Diagnostics;
9-
using System.IO;
10-
using System.IO.Pipelines;
119
using System.Threading;
1210
using System.Threading.Tasks;
1311
using Microsoft.CodeAnalysis.ErrorReporting;
1412
using Microsoft.ServiceHub.Framework;
15-
using Microsoft.ServiceHub.Framework.Services;
16-
using Microsoft.VisualStudio.Threading;
17-
using Nerdbank.Streams;
1813
using Roslyn.Utilities;
1914

2015
namespace Microsoft.CodeAnalysis.Remote
@@ -28,7 +23,6 @@ internal abstract partial class BrokeredServiceBase : IDisposable
2823
protected readonly RemoteWorkspaceManager WorkspaceManager;
2924

3025
protected readonly SolutionAssetSource SolutionAssetSource;
31-
protected readonly CancellationTokenSource ClientDisconnectedSource;
3226
protected readonly ServiceBrokerClient ServiceBrokerClient;
3327

3428
// test data are only available when running tests:
@@ -49,8 +43,7 @@ protected BrokeredServiceBase(in ServiceConstructionArguments arguments)
4943
ServiceBrokerClient = new ServiceBrokerClient(arguments.ServiceBroker);
5044
#pragma warning restore
5145

52-
SolutionAssetSource = new SolutionAssetSource(ServiceBrokerClient, arguments.ClientDisconnectedSource);
53-
ClientDisconnectedSource = arguments.ClientDisconnectedSource;
46+
SolutionAssetSource = new SolutionAssetSource(ServiceBrokerClient);
5447
}
5548

5649
public void Dispose()
@@ -72,11 +65,10 @@ protected Task<Solution> GetSolutionAsync(PinnedSolutionInfo solutionInfo, Cance
7265
protected async ValueTask<T> RunServiceAsync<T>(Func<CancellationToken, ValueTask<T>> implementation, CancellationToken cancellationToken)
7366
{
7467
WorkspaceManager.SolutionAssetCache.UpdateLastActivityTime();
75-
using var combined = cancellationToken.CombineWith(ClientDisconnectedSource.Token);
7668

7769
try
7870
{
79-
return await implementation(combined.Token).ConfigureAwait(false);
71+
return await implementation(cancellationToken).ConfigureAwait(false);
8072
}
8173
catch (Exception ex) when (FatalError.ReportWithoutCrashUnlessCanceledAndPropagate(ex, cancellationToken))
8274
{
@@ -87,11 +79,10 @@ protected async ValueTask<T> RunServiceAsync<T>(Func<CancellationToken, ValueTas
8779
protected async ValueTask RunServiceAsync(Func<CancellationToken, ValueTask> implementation, CancellationToken cancellationToken)
8880
{
8981
WorkspaceManager.SolutionAssetCache.UpdateLastActivityTime();
90-
using var combined = cancellationToken.CombineWith(ClientDisconnectedSource.Token);
9182

9283
try
9384
{
94-
await implementation(combined.Token).ConfigureAwait(false);
85+
await implementation(cancellationToken).ConfigureAwait(false);
9586
}
9687
catch (Exception ex) when (FatalError.ReportWithoutCrashUnlessCanceledAndPropagate(ex, cancellationToken))
9788
{

‎src/Workspaces/Remote/ServiceHub/Services/DesignerAttributeDiscovery/RemoteDesignerAttributeIncrementalAnalyzer.cs‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,16 @@ public RemoteDesignerAttributeIncrementalAnalyzer(Workspace workspace, RemoteCal
2727

2828
protected override async ValueTask ReportProjectRemovedAsync(ProjectId projectId, CancellationToken cancellationToken)
2929
{
30-
// cancel whenever the analyzer runner cancels or the client disconnects and the request is canceled:
31-
using var linkedSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, _callback.ClientDisconnectedSource.Token);
32-
3330
await _callback.InvokeAsync(
3431
(callback, cancellationToken) => callback.OnProjectRemovedAsync(projectId, cancellationToken),
35-
linkedSource.Token).ConfigureAwait(false);
32+
cancellationToken).ConfigureAwait(false);
3633
}
3734

3835
protected override async ValueTask ReportDesignerAttributeDataAsync(List<DesignerAttributeData> data, CancellationToken cancellationToken)
3936
{
40-
// cancel whenever the analyzer runner cancels or the client disconnects and the request is canceled:
41-
using var linkedSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, _callback.ClientDisconnectedSource.Token);
42-
4337
await _callback.InvokeAsync(
4438
(callback, cancellationToken) => callback.ReportDesignerAttributeDataAsync(data.ToImmutableArray(), cancellationToken),
45-
linkedSource.Token).ConfigureAwait(false);
39+
cancellationToken).ConfigureAwait(false);
4640
}
4741
}
4842
}

‎src/Workspaces/Remote/ServiceHub/Services/ProjectTelemetry/RemoteProjectTelemetryIncrementalAnalyzer.cs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
using System.Threading.Tasks;
1010
using Microsoft.CodeAnalysis.ProjectTelemetry;
1111
using Microsoft.CodeAnalysis.SolutionCrawler;
12-
using StreamJsonRpc;
1312

1413
namespace Microsoft.CodeAnalysis.Remote
1514
{
@@ -67,12 +66,9 @@ public override async Task AnalyzeProjectAsync(Project project, bool semanticsCh
6766
_projectToData[projectId] = info;
6867
}
6968

70-
// cancel whenever the analyzer runner cancels or the client disconnects and the request is canceled:
71-
using var linkedSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, _callback.ClientDisconnectedSource.Token);
72-
7369
await _callback.InvokeAsync(
7470
(callback, cancellationToken) => callback.ReportProjectTelemetryDataAsync(info, cancellationToken),
75-
linkedSource.Token).ConfigureAwait(false);
71+
cancellationToken).ConfigureAwait(false);
7672
}
7773

7874
public override Task RemoveProjectAsync(ProjectId projectId, CancellationToken cancellationToken)

‎src/Workspaces/Remote/ServiceHub/Services/TodoCommentsDiscovery/RemoteTodoCommentsIncrementalAnalyzer.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,9 @@ public RemoteTodoCommentsIncrementalAnalyzer(RemoteCallback<ITodoCommentsListene
2323

2424
protected override async ValueTask ReportTodoCommentDataAsync(DocumentId documentId, ImmutableArray<TodoCommentData> data, CancellationToken cancellationToken)
2525
{
26-
// cancel whenever the analyzer runner cancels or the client disconnects and the request is canceled:
27-
using var linkedSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, _callback.ClientDisconnectedSource.Token);
28-
2926
await _callback.InvokeAsync(
3027
(callback, cancellationToken) => callback.ReportTodoCommentDataAsync(documentId, data, cancellationToken),
31-
linkedSource.Token).ConfigureAwait(false);
28+
cancellationToken).ConfigureAwait(false);
3229
}
3330
}
3431
}

0 commit comments

Comments
 (0)