diff --git a/src/Workspaces/Remote/Core/BrokeredServiceConnection.cs b/src/Workspaces/Remote/Core/BrokeredServiceConnection.cs index 1c697fe1a54f9..96b8c63138a09 100644 --- a/src/Workspaces/Remote/Core/BrokeredServiceConnection.cs +++ b/src/Workspaces/Remote/Core/BrokeredServiceConnection.cs @@ -286,18 +286,12 @@ internal static async ValueTask InvokeStreamingServiceAsync( // After this point, the full cancellation sequence is as follows: // 1. 'cancellationToken' indicates cancellation is requested - // 2. 'invocation' has cancellation requested + // 2. 'invocation' and 'readerTask' have cancellation requested // 3. 'invocation' stops writing to 'pipe.Writer' - // 4. 'readerTask' has cancellation requested - // 5. 'readerTask' stops reading from 'pipe.Reader' - // 6. 'pipe.Writer' is completed - // 7. 'pipe.Reader' is completed - // 8. OperationCanceledException is thrown back to the caller - - // Create a separate cancellation token for the reader, which we keep open until after the call to invoke - // completes. If we close the reader before cancellation is processed by the remote call, it might block - // (deadlock) while writing to a stream which is no longer processing data. - using var readerCancellationSource = new CancellationTokenSource(); + // 4. 'pipe.Writer' is completed + // 5. 'readerTask' continues reading until EndOfStreamException (workaround for https://github.com/AArnott/Nerdbank.Streams/issues/361) + // 6. 'pipe.Reader' is completed + // 7. OperationCanceledException is thrown back to the caller var pipe = new Pipe(); @@ -312,11 +306,6 @@ internal static async ValueTask InvokeStreamingServiceAsync( } catch (Exception e) { - // Make sure the reader is aware of a cancellation request before completing the writer. Otherwise, - // the reader could attempt to read past the end of a completed stream without realizing that - // cancellation is expected. - readerCancellationSource.Cancel(); - // Ensure that the writer is complete if an exception is thrown // before the writer is passed to the RPC proxy. Once it's passed to the proxy // the proxy should complete it as soon as the remote side completes it. @@ -333,7 +322,7 @@ internal static async ValueTask InvokeStreamingServiceAsync( try { - return await reader(pipe.Reader, readerCancellationSource.Token).ConfigureAwait(false); + return await reader(pipe.Reader, cancellationToken).ConfigureAwait(false); } catch (Exception e) when ((exception = e) == null) { @@ -343,7 +332,7 @@ internal static async ValueTask InvokeStreamingServiceAsync( { await pipe.Reader.CompleteAsync(exception).ConfigureAwait(false); } - }, readerCancellationSource.Token); + }, mustNotCancelToken); await Task.WhenAll(writerTask, readerTask).ConfigureAwait(false); diff --git a/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs b/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs index 1948dee5f0c27..c5067968283cc 100644 --- a/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs +++ b/src/Workspaces/Remote/Core/RemoteHostAssetSerialization.cs @@ -94,6 +94,9 @@ static void WriteAsset(ObjectWriter writer, ISerializerService serializer, Solut cancellationToken.ThrowIfCancellationRequested(); var mustNotCancelToken = CancellationToken.None; + // Workaround for https://github.com/AArnott/Nerdbank.Streams/issues/361 + var mustNotCancelUntilBugFix = CancellationToken.None; + // Workaround for ObjectReader not supporting async reading. // Unless we read from the RPC stream asynchronously and with cancallation support we might deadlock when the server cancels. // https://github.com/dotnet/roslyn/issues/47861 @@ -108,7 +111,7 @@ static void WriteAsset(ObjectWriter writer, ISerializerService serializer, Solut { try { - await pipeReader.CopyToAsync(localPipe.Writer, cancellationToken).ConfigureAwait(false); + await pipeReader.CopyToAsync(localPipe.Writer, mustNotCancelUntilBugFix).ConfigureAwait(false); } catch (Exception e) { @@ -124,7 +127,7 @@ static void WriteAsset(ObjectWriter writer, ISerializerService serializer, Solut try { using var stream = localPipe.Reader.AsStream(leaveOpen: false); - return ReadData(stream, scopeId, checksums, serializerService, cancellationToken); + return ReadData(stream, scopeId, checksums, serializerService, mustNotCancelUntilBugFix); } catch (EndOfStreamException) when (IsEndOfStreamExceptionExpected(copyException, cancellationToken)) { diff --git a/src/Workspaces/Remote/Core/SolutionAssetProvider.cs b/src/Workspaces/Remote/Core/SolutionAssetProvider.cs index 3666e6085dbbf..5323a236e5866 100644 --- a/src/Workspaces/Remote/Core/SolutionAssetProvider.cs +++ b/src/Workspaces/Remote/Core/SolutionAssetProvider.cs @@ -102,17 +102,7 @@ async Task CopyPipeDataAsync() finally { await localPipe.Reader.CompleteAsync(exception).ConfigureAwait(false); - - if (cancellationToken.IsCancellationRequested && exception is null or OperationCanceledException) - { - // Throw rather than close the pipe writer. The caller will coordinate cancellation and closing - // of the reader and writer together. - cancellationToken.ThrowIfCancellationRequested(); - } - else - { - await pipeWriter.CompleteAsync(exception).ConfigureAwait(false); - } + await pipeWriter.CompleteAsync(exception).ConfigureAwait(false); } } }