From 9b437acec9fa8f88669d34690983a648fa0bcde5 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Tue, 2 Nov 2021 08:23:20 -0600 Subject: [PATCH] Do not propagate a CancellationToken from a method arg to a long-lived state machine This fixes #414, which is a hang that occurs when the `CancellationToken` passed to `MultiplexingStream.CreateAsync` is cancelled after `CreateAsync` has completed. --- .../MultiplexingStreamTests.cs | 19 +++++++++++++++++++ src/Nerdbank.Streams/MultiplexingStream.cs | 4 +++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/Nerdbank.Streams.Tests/MultiplexingStreamTests.cs b/src/Nerdbank.Streams.Tests/MultiplexingStreamTests.cs index 082eebff..f7409650 100644 --- a/src/Nerdbank.Streams.Tests/MultiplexingStreamTests.cs +++ b/src/Nerdbank.Streams.Tests/MultiplexingStreamTests.cs @@ -307,6 +307,25 @@ public async Task CreateChannelAsync_CanceledBeforeAcceptance() await Assert.ThrowsAnyAsync(() => channel1Task).WithCancellation(this.TimeoutToken); } + [Fact] + public async Task CreateAsync_CancellationToken() + { + var cts = new CancellationTokenSource(); + var streamPair = FullDuplexStream.CreatePair(); + Task mx1Task = MultiplexingStream.CreateAsync(streamPair.Item1, new MultiplexingStream.Options { ProtocolMajorVersion = this.ProtocolMajorVersion }, cts.Token); + Task mx2Task = MultiplexingStream.CreateAsync(streamPair.Item2, new MultiplexingStream.Options { ProtocolMajorVersion = this.ProtocolMajorVersion }, CancellationToken.None); + MultiplexingStream mx1 = await mx1Task; + MultiplexingStream mx2 = await mx2Task; + + // At this point the cancellation token really shouldn't have any effect on mx1 now that the connection is established. + cts.Cancel(); + + Task ch1Task = mx1.OfferChannelAsync(string.Empty, this.TimeoutToken); + Task ch2Task = mx2.AcceptChannelAsync(string.Empty, this.TimeoutToken); + + await Task.WhenAll(ch1Task, ch2Task).WithCancellation(this.TimeoutToken); + } + [Fact] public async Task CreateChannelAsync() { diff --git a/src/Nerdbank.Streams/MultiplexingStream.cs b/src/Nerdbank.Streams/MultiplexingStream.cs index 0cf11805..120f9a29 100644 --- a/src/Nerdbank.Streams/MultiplexingStream.cs +++ b/src/Nerdbank.Streams/MultiplexingStream.cs @@ -284,7 +284,9 @@ public static async Task CreateAsync(Stream stream, Options? throw new NotSupportedException(Strings.SeededChannelsRequireV3Protocol); } - var streamWriter = stream.UsePipeWriter(cancellationToken: cancellationToken); + // Do NOT specify our own cancellationToken parameter in UsePipeWriter, since this PipeWriter + // must outlive this method and therefore should not be canceled later if that token is eventually canceled. + var streamWriter = stream.UsePipeWriter(cancellationToken: CancellationToken.None); var formatter = options.ProtocolMajorVersion switch {