From 07037acb3693c96fa2fb3cb1c8fc4a3160a291e8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 01:39:47 +0000 Subject: [PATCH 01/13] Initial plan From 3971d2ca8e416b09820f6c9713fa08a98ced1df3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 01:45:01 +0000 Subject: [PATCH 02/13] Add HTTP streaming abort test Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Http/HttpStreamingAbortTests.4.1.0.cs | 202 ++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs diff --git a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs new file mode 100644 index 00000000000..a94828b941d --- /dev/null +++ b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs @@ -0,0 +1,202 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.IO; +using System.ServiceModel; +using System.ServiceModel.Channels; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Infrastructure.Common; +using Xunit; + +public static class HttpStreamingAbortTests +{ + [WcfFact] + [OuterLoop] + public static void HttpStreaming_Abort_During_Response_Receiving() + { + // This test validates that calling Abort() on an HTTP channel works correctly + // when the channel is in the middle of receiving a streamed response. + // If Abort() doesn't propagate correctly, the test will timeout with a TimeoutException. + // If Abort() works correctly, a CommunicationObjectAbortedException should be thrown. + + ChannelFactory factory = null; + IWcfService serviceProxy = null; + CustomBinding binding = null; + Stream responseStream = null; + Exception caughtException = null; + + try + { + // *** SETUP *** \\ + // Create a binding with streamed transfer mode + binding = new CustomBinding( + new TextMessageEncodingBindingElement(), + new HttpTransportBindingElement + { + TransferMode = TransferMode.StreamedResponse, + // Set a short SendTimeout so that if Abort() doesn't work, + // we'll get a TimeoutException relatively quickly + MaxReceivedMessageSize = 1024 * 1024 // 1 MB + }); + + // Set a reasonable SendTimeout - if Abort() doesn't work, this will cause a timeout + binding.SendTimeout = TimeSpan.FromSeconds(10); + + factory = new ChannelFactory(binding, new EndpointAddress(Endpoints.CustomTextEncoderStreamed_Address)); + serviceProxy = factory.CreateChannel(); + + // Create a large string to ensure the response takes time to read + string testString = new string('a', 500000); // 500KB of data + + // *** EXECUTE *** \\ + // Start the call to get a stream response + responseStream = serviceProxy.GetStreamFromString(testString); + + // Start reading a small amount from the stream to ensure we're in the receiving phase + byte[] buffer = new byte[1024]; + int bytesRead = responseStream.Read(buffer, 0, buffer.Length); + + // Verify we actually received some data + Assert.True(bytesRead > 0, "Expected to read some data from the stream"); + + // Now abort the channel while we're in the middle of receiving the response + // This should cause the ongoing read operation to be cancelled + ((ICommunicationObject)serviceProxy).Abort(); + + // Try to continue reading from the stream + // If Abort() works correctly, this should throw an exception + // If Abort() doesn't work, this will hang until the SendTimeout expires + try + { + while (responseStream.Read(buffer, 0, buffer.Length) > 0) + { + // Keep reading + } + } + catch (Exception ex) + { + caughtException = ex; + } + + // *** VALIDATE *** \\ + // We expect an exception to be thrown after Abort() is called + Assert.NotNull(caughtException); + + // The exception should be related to the communication object being aborted + // It could be CommunicationObjectAbortedException or an IOException wrapping it + string exceptionType = caughtException.GetType().Name; + Assert.True( + exceptionType == "CommunicationObjectAbortedException" || + exceptionType == "IOException" || + exceptionType == "CommunicationException", + $"Expected CommunicationObjectAbortedException, IOException, or CommunicationException, but got: {exceptionType}"); + } + catch (TimeoutException) + { + // If we get a TimeoutException, it means Abort() didn't work correctly + Assert.Fail("Test timed out, which indicates that Abort() did not properly cancel the ongoing stream read operation."); + } + finally + { + // *** ENSURE CLEANUP *** \\ + responseStream?.Dispose(); + ScenarioTestHelpers.CloseCommunicationObjects((ICommunicationObject)serviceProxy, factory); + } + } + + [WcfFact] + [OuterLoop] + public static async Task HttpStreaming_Abort_During_Async_Response_Receiving() + { + // This test validates that calling Abort() on an HTTP channel works correctly + // when the channel is in the middle of receiving a streamed response asynchronously. + // If Abort() doesn't propagate correctly, the test will timeout with a TimeoutException. + // If Abort() works correctly, a CommunicationObjectAbortedException should be thrown. + + ChannelFactory factory = null; + IWcfService serviceProxy = null; + CustomBinding binding = null; + Stream responseStream = null; + Exception caughtException = null; + + try + { + // *** SETUP *** \\ + // Create a binding with streamed transfer mode + binding = new CustomBinding( + new TextMessageEncodingBindingElement(), + new HttpTransportBindingElement + { + TransferMode = TransferMode.StreamedResponse, + MaxReceivedMessageSize = 1024 * 1024 // 1 MB + }); + + // Set a reasonable SendTimeout - if Abort() doesn't work, this will cause a timeout + binding.SendTimeout = TimeSpan.FromSeconds(10); + + factory = new ChannelFactory(binding, new EndpointAddress(Endpoints.CustomTextEncoderStreamed_Address)); + serviceProxy = factory.CreateChannel(); + + // Create a large string to ensure the response takes time to read + string testString = new string('a', 500000); // 500KB of data + + // *** EXECUTE *** \\ + // Start the call to get a stream response + responseStream = serviceProxy.GetStreamFromString(testString); + + // Start reading a small amount from the stream to ensure we're in the receiving phase + byte[] buffer = new byte[1024]; + int bytesRead = await responseStream.ReadAsync(buffer, 0, buffer.Length); + + // Verify we actually received some data + Assert.True(bytesRead > 0, "Expected to read some data from the stream"); + + // Now abort the channel while we're in the middle of receiving the response + ((ICommunicationObject)serviceProxy).Abort(); + + // Try to continue reading from the stream asynchronously + // If Abort() works correctly, this should throw an exception + // If Abort() doesn't work, this will hang until the SendTimeout expires + try + { + while ((await responseStream.ReadAsync(buffer, 0, buffer.Length)) > 0) + { + // Keep reading + } + } + catch (Exception ex) + { + caughtException = ex; + } + + // *** VALIDATE *** \\ + // We expect an exception to be thrown after Abort() is called + Assert.NotNull(caughtException); + + // The exception should be related to the communication object being aborted + // It could be CommunicationObjectAbortedException or an IOException wrapping it + string exceptionType = caughtException.GetType().Name; + Assert.True( + exceptionType == "CommunicationObjectAbortedException" || + exceptionType == "IOException" || + exceptionType == "CommunicationException" || + exceptionType == "OperationCanceledException", + $"Expected CommunicationObjectAbortedException, IOException, CommunicationException, or OperationCanceledException, but got: {exceptionType}"); + } + catch (TimeoutException) + { + // If we get a TimeoutException, it means Abort() didn't work correctly + Assert.Fail("Test timed out, which indicates that Abort() did not properly cancel the ongoing stream read operation."); + } + finally + { + // *** ENSURE CLEANUP *** \\ + responseStream?.Dispose(); + ScenarioTestHelpers.CloseCommunicationObjects((ICommunicationObject)serviceProxy, factory); + } + } +} From 95e00665bc6372b420acba408c09507f568e3afd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 01:47:01 +0000 Subject: [PATCH 03/13] Improve exception type checking using pattern matching Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Http/HttpStreamingAbortTests.4.1.0.cs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs index a94828b941d..1c410f1db45 100644 --- a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs +++ b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs @@ -88,12 +88,11 @@ public static void HttpStreaming_Abort_During_Response_Receiving() // The exception should be related to the communication object being aborted // It could be CommunicationObjectAbortedException or an IOException wrapping it - string exceptionType = caughtException.GetType().Name; Assert.True( - exceptionType == "CommunicationObjectAbortedException" || - exceptionType == "IOException" || - exceptionType == "CommunicationException", - $"Expected CommunicationObjectAbortedException, IOException, or CommunicationException, but got: {exceptionType}"); + caughtException is CommunicationObjectAbortedException || + caughtException is IOException || + caughtException is CommunicationException, + $"Expected CommunicationObjectAbortedException, IOException, or CommunicationException, but got: {caughtException.GetType().Name}"); } catch (TimeoutException) { @@ -179,13 +178,12 @@ public static async Task HttpStreaming_Abort_During_Async_Response_Receiving() // The exception should be related to the communication object being aborted // It could be CommunicationObjectAbortedException or an IOException wrapping it - string exceptionType = caughtException.GetType().Name; Assert.True( - exceptionType == "CommunicationObjectAbortedException" || - exceptionType == "IOException" || - exceptionType == "CommunicationException" || - exceptionType == "OperationCanceledException", - $"Expected CommunicationObjectAbortedException, IOException, CommunicationException, or OperationCanceledException, but got: {exceptionType}"); + caughtException is CommunicationObjectAbortedException || + caughtException is IOException || + caughtException is CommunicationException || + caughtException is OperationCanceledException, + $"Expected CommunicationObjectAbortedException, IOException, CommunicationException, or OperationCanceledException, but got: {caughtException.GetType().Name}"); } catch (TimeoutException) { From 67d5b234dcf29135d8922861c46ac1785c7b1319 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 01:48:13 +0000 Subject: [PATCH 04/13] Fix misleading comment about timeout configuration Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs index 1c410f1db45..ffe77d68e35 100644 --- a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs +++ b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs @@ -38,8 +38,6 @@ public static void HttpStreaming_Abort_During_Response_Receiving() new HttpTransportBindingElement { TransferMode = TransferMode.StreamedResponse, - // Set a short SendTimeout so that if Abort() doesn't work, - // we'll get a TimeoutException relatively quickly MaxReceivedMessageSize = 1024 * 1024 // 1 MB }); From 7c8a85b106334d81a2de6d564a6ca734f98b5bb8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 01:49:48 +0000 Subject: [PATCH 05/13] Fix timeout configuration to use ReceiveTimeout instead of SendTimeout Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Binding/Http/HttpStreamingAbortTests.4.1.0.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs index ffe77d68e35..fa8890ec4a3 100644 --- a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs +++ b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs @@ -41,8 +41,8 @@ public static void HttpStreaming_Abort_During_Response_Receiving() MaxReceivedMessageSize = 1024 * 1024 // 1 MB }); - // Set a reasonable SendTimeout - if Abort() doesn't work, this will cause a timeout - binding.SendTimeout = TimeSpan.FromSeconds(10); + // Set a reasonable ReceiveTimeout - if Abort() doesn't work, this will cause a timeout + binding.ReceiveTimeout = TimeSpan.FromSeconds(10); factory = new ChannelFactory(binding, new EndpointAddress(Endpoints.CustomTextEncoderStreamed_Address)); serviceProxy = factory.CreateChannel(); @@ -132,8 +132,8 @@ public static async Task HttpStreaming_Abort_During_Async_Response_Receiving() MaxReceivedMessageSize = 1024 * 1024 // 1 MB }); - // Set a reasonable SendTimeout - if Abort() doesn't work, this will cause a timeout - binding.SendTimeout = TimeSpan.FromSeconds(10); + // Set a reasonable ReceiveTimeout - if Abort() doesn't work, this will cause a timeout + binding.ReceiveTimeout = TimeSpan.FromSeconds(10); factory = new ChannelFactory(binding, new EndpointAddress(Endpoints.CustomTextEncoderStreamed_Address)); serviceProxy = factory.CreateChannel(); From 3bd890d86ede15ec931bc91dcb0b122080db7ca5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 01:51:06 +0000 Subject: [PATCH 06/13] Update comments to reference ReceiveTimeout instead of SendTimeout Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs index fa8890ec4a3..3dfdcfa2420 100644 --- a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs +++ b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs @@ -67,7 +67,7 @@ public static void HttpStreaming_Abort_During_Response_Receiving() // Try to continue reading from the stream // If Abort() works correctly, this should throw an exception - // If Abort() doesn't work, this will hang until the SendTimeout expires + // If Abort() doesn't work, this will hang until the ReceiveTimeout expires try { while (responseStream.Read(buffer, 0, buffer.Length) > 0) @@ -157,7 +157,7 @@ public static async Task HttpStreaming_Abort_During_Async_Response_Receiving() // Try to continue reading from the stream asynchronously // If Abort() works correctly, this should throw an exception - // If Abort() doesn't work, this will hang until the SendTimeout expires + // If Abort() doesn't work, this will hang until the ReceiveTimeout expires try { while ((await responseStream.ReadAsync(buffer, 0, buffer.Length)) > 0) From 4aba311d5801bea3fb6ee6ed03a8b7c6493c177b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 01:52:44 +0000 Subject: [PATCH 07/13] Extract magic numbers as constants for better maintainability Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Binding/Http/HttpStreamingAbortTests.4.1.0.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs index 3dfdcfa2420..83207e2632b 100644 --- a/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs +++ b/src/System.Private.ServiceModel/tests/Scenarios/Binding/Http/HttpStreamingAbortTests.4.1.0.cs @@ -14,6 +14,9 @@ public static class HttpStreamingAbortTests { + private const int BufferSize = 1024; + private const int LargeStreamSize = 500000; // 500KB + [WcfFact] [OuterLoop] public static void HttpStreaming_Abort_During_Response_Receiving() @@ -48,14 +51,14 @@ public static void HttpStreaming_Abort_During_Response_Receiving() serviceProxy = factory.CreateChannel(); // Create a large string to ensure the response takes time to read - string testString = new string('a', 500000); // 500KB of data + string testString = new string('a', LargeStreamSize); // *** EXECUTE *** \\ // Start the call to get a stream response responseStream = serviceProxy.GetStreamFromString(testString); // Start reading a small amount from the stream to ensure we're in the receiving phase - byte[] buffer = new byte[1024]; + byte[] buffer = new byte[BufferSize]; int bytesRead = responseStream.Read(buffer, 0, buffer.Length); // Verify we actually received some data @@ -139,14 +142,14 @@ public static async Task HttpStreaming_Abort_During_Async_Response_Receiving() serviceProxy = factory.CreateChannel(); // Create a large string to ensure the response takes time to read - string testString = new string('a', 500000); // 500KB of data + string testString = new string('a', LargeStreamSize); // *** EXECUTE *** \\ // Start the call to get a stream response responseStream = serviceProxy.GetStreamFromString(testString); // Start reading a small amount from the stream to ensure we're in the receiving phase - byte[] buffer = new byte[1024]; + byte[] buffer = new byte[BufferSize]; int bytesRead = await responseStream.ReadAsync(buffer, 0, buffer.Length); // Verify we actually received some data From d296c3f45784ac878b6e197ab788512fde870109 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 02:02:04 +0000 Subject: [PATCH 08/13] Implement fix to propagate Abort() to HTTP response stream reading - Added CancellationToken parameter to HttpResponseMessageHelper constructor - Created GetCombinedCancellationTokenAsync helper to combine timeout and abort tokens - Updated all stream reading operations to use combined cancellation token - Pass _httpSendCts.Token from HttpClientChannelAsyncRequest to enable abort during response reception Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Channels/HttpChannelFactory.cs | 2 +- .../Channels/HttpResponseMessageHelper.cs | 36 ++++++++++++++++--- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpChannelFactory.cs b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpChannelFactory.cs index 12d61ce6348..b00123ca822 100644 --- a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpChannelFactory.cs +++ b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpChannelFactory.cs @@ -1131,7 +1131,7 @@ public async Task ReceiveReplyAsync(TimeoutHelper timeoutHelper) try { _timeoutHelper = timeoutHelper; - var responseHelper = new HttpResponseMessageHelper(_httpResponseMessage, _factory); + var responseHelper = new HttpResponseMessageHelper(_httpResponseMessage, _factory, _httpSendCts.Token); var replyMessage = await responseHelper.ParseIncomingResponse(timeoutHelper); TryCompleteHttpRequest(_httpRequestMessage); return replyMessage; diff --git a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs index 3c292bad425..46f04f585a2 100644 --- a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs +++ b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs @@ -21,10 +21,11 @@ internal class HttpResponseMessageHelper private readonly MessageEncoder _encoder; private readonly HttpRequestMessage _httpRequestMessage; private readonly HttpResponseMessage _httpResponseMessage; + private readonly CancellationToken _abortToken; private string _contentType; private long _contentLength; - public HttpResponseMessageHelper(HttpResponseMessage httpResponseMessage, HttpChannelFactory factory) + public HttpResponseMessageHelper(HttpResponseMessage httpResponseMessage, HttpChannelFactory factory, CancellationToken abortToken = default) { Contract.Assert(httpResponseMessage != null); Contract.Assert(httpResponseMessage.RequestMessage != null); @@ -33,6 +34,7 @@ public HttpResponseMessageHelper(HttpResponseMessage httpResponseMessage, HttpCh _httpRequestMessage = httpResponseMessage.RequestMessage; _factory = factory; _encoder = factory.MessageEncoderFactory.Encoder; + _abortToken = abortToken; } internal async Task ParseIncomingResponse(TimeoutHelper timeoutHelper) @@ -188,7 +190,7 @@ private async Task ReadChunkedBufferedMessageAsync(Task inputSt { try { - return await _encoder.ReadMessageAsync(await inputStreamTask, _factory.BufferManager, _factory.MaxBufferSize, _contentType, await timeoutHelper.GetCancellationTokenAsync()); + return await _encoder.ReadMessageAsync(await inputStreamTask, _factory.BufferManager, _factory.MaxBufferSize, _contentType, await GetCombinedCancellationTokenAsync(timeoutHelper)); } catch (XmlException xmlException) { @@ -212,7 +214,7 @@ private async Task ReadBufferedMessageAsync(Task inputStreamTas byte[] buffer = messageBuffer.Array; int offset = 0; int count = messageBuffer.Count; - var ct = await timeoutHelper.GetCancellationTokenAsync(); + var ct = await GetCombinedCancellationTokenAsync(timeoutHelper); while (count > 0) { @@ -273,7 +275,7 @@ private async Task DecodeBufferedMessageAsync(ArraySegment buffer { try { - var ct = await timeoutHelper.GetCancellationTokenAsync(); + var ct = await GetCombinedCancellationTokenAsync(timeoutHelper); // if we're chunked, make sure we've consumed the whole body if (_contentLength == -1 && buffer.Count == _factory.MaxReceivedMessageSize) { @@ -304,6 +306,30 @@ private async Task DecodeBufferedMessageAsync(ArraySegment buffer } } + private async Task GetCombinedCancellationTokenAsync(TimeoutHelper timeoutHelper) + { + var timeoutToken = await timeoutHelper.GetCancellationTokenAsync(); + + // If no abort token is provided, just use the timeout token + if (_abortToken == default || !_abortToken.CanBeCanceled) + { + return timeoutToken; + } + + // If the timeout token can't be cancelled, just use the abort token + if (!timeoutToken.CanBeCanceled) + { + return _abortToken; + } + + // Both tokens can be cancelled, so create a linked token source + // Note: The caller is responsible for disposing the linked token source if needed + // However, since we're using it for short-lived operations within this helper, + // we'll rely on the garbage collector + var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutToken, _abortToken); + return linkedCts.Token; + } + private async Task GetStreamAsync(TimeoutHelper timeoutHelper) { var content = _httpResponseMessage.Content; @@ -313,7 +339,7 @@ private async Task GetStreamAsync(TimeoutHelper timeoutHelper) { contentStream = await content.ReadAsStreamAsync(); _contentLength = content.Headers.ContentLength.HasValue ? content.Headers.ContentLength.Value : -1; - var cancellationToken = await timeoutHelper.GetCancellationTokenAsync(); + var cancellationToken = await GetCombinedCancellationTokenAsync(timeoutHelper); if (_contentLength <= 0) { var preReadBuffer = new byte[1]; From eb36723ce61eeff7de47303f8265486d10af5c93 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 02:04:46 +0000 Subject: [PATCH 09/13] Address code review feedback on CancellationTokenSource disposal - Added detailed comments explaining why the linked CTS is not explicitly disposed - Clarified that HttpResponseMessageHelper has short lifetime and CTS will be garbage collected - Fixed CancellationToken default comparison to use CanBeCanceled check Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Channels/HttpResponseMessageHelper.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs index 46f04f585a2..52d73998781 100644 --- a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs +++ b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs @@ -310,8 +310,8 @@ private async Task GetCombinedCancellationTokenAsync(TimeoutH { var timeoutToken = await timeoutHelper.GetCancellationTokenAsync(); - // If no abort token is provided, just use the timeout token - if (_abortToken == default || !_abortToken.CanBeCanceled) + // If no abort token is provided or it can't be cancelled, just use the timeout token + if (!_abortToken.CanBeCanceled) { return timeoutToken; } @@ -323,9 +323,11 @@ private async Task GetCombinedCancellationTokenAsync(TimeoutH } // Both tokens can be cancelled, so create a linked token source - // Note: The caller is responsible for disposing the linked token source if needed - // However, since we're using it for short-lived operations within this helper, - // we'll rely on the garbage collector + // Note: This creates a CancellationTokenSource that is not explicitly disposed. + // However, the HttpResponseMessageHelper has a short lifetime (single response parsing), + // and the CTS will be garbage collected when the helper goes out of scope. + // The registrations within the linked CTS will be automatically cleaned up when + // either source token is cancelled or the CTS is finalized. var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutToken, _abortToken); return linkedCts.Token; } From 284f05795e0adfcdba4cd4dbe097027eebb5aef5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 02:07:04 +0000 Subject: [PATCH 10/13] Cache combined cancellation token to avoid repeated allocations - Added _linkedCts and _cachedCombinedToken fields to HttpResponseMessageHelper - Modified GetCombinedCancellationTokenAsync to cache the combined token on first call - This avoids creating multiple linked CancellationTokenSource instances - Improves resource management by creating only one CTS per helper instance Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Channels/HttpResponseMessageHelper.cs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs index 52d73998781..97a558e793c 100644 --- a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs +++ b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs @@ -24,6 +24,8 @@ internal class HttpResponseMessageHelper private readonly CancellationToken _abortToken; private string _contentType; private long _contentLength; + private CancellationTokenSource _linkedCts; + private CancellationToken? _cachedCombinedToken; public HttpResponseMessageHelper(HttpResponseMessage httpResponseMessage, HttpChannelFactory factory, CancellationToken abortToken = default) { @@ -308,28 +310,33 @@ private async Task DecodeBufferedMessageAsync(ArraySegment buffer private async Task GetCombinedCancellationTokenAsync(TimeoutHelper timeoutHelper) { + // If we've already computed the combined token, return it + if (_cachedCombinedToken.HasValue) + { + return _cachedCombinedToken.Value; + } + var timeoutToken = await timeoutHelper.GetCancellationTokenAsync(); // If no abort token is provided or it can't be cancelled, just use the timeout token if (!_abortToken.CanBeCanceled) { + _cachedCombinedToken = timeoutToken; return timeoutToken; } // If the timeout token can't be cancelled, just use the abort token if (!timeoutToken.CanBeCanceled) { + _cachedCombinedToken = _abortToken; return _abortToken; } // Both tokens can be cancelled, so create a linked token source - // Note: This creates a CancellationTokenSource that is not explicitly disposed. - // However, the HttpResponseMessageHelper has a short lifetime (single response parsing), - // and the CTS will be garbage collected when the helper goes out of scope. - // The registrations within the linked CTS will be automatically cleaned up when - // either source token is cancelled or the CTS is finalized. - var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutToken, _abortToken); - return linkedCts.Token; + // Store the CTS so we can dispose it later if needed + _linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutToken, _abortToken); + _cachedCombinedToken = _linkedCts.Token; + return _linkedCts.Token; } private async Task GetStreamAsync(TimeoutHelper timeoutHelper) From a19fad8480f74264043381e5830160d2a36d5874 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 23:24:48 +0000 Subject: [PATCH 11/13] Refactor to use UnsafeRegister pattern instead of linked CancellationTokenSource - Changed HttpResponseMessageHelper to accept CancellationTokenSource instead of CancellationToken - Added s_cancelCts callback in HttpResponseMessageHelper (same pattern as in HttpChannelFactory) - Register timeout token callback in ParseIncomingResponse using UnsafeRegister - Removed linked CancellationTokenSource creation and caching logic - Simplified GetCancellationTokenAsync to just return _httpSendCts.Token or timeout token - This matches the pattern already used in SendRequestAsync and avoids resource management issues Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Channels/HttpChannelFactory.cs | 2 +- .../Channels/HttpResponseMessageHelper.cs | 106 ++++++++++-------- 2 files changed, 58 insertions(+), 50 deletions(-) diff --git a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpChannelFactory.cs b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpChannelFactory.cs index b00123ca822..5b83f121532 100644 --- a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpChannelFactory.cs +++ b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpChannelFactory.cs @@ -1131,7 +1131,7 @@ public async Task ReceiveReplyAsync(TimeoutHelper timeoutHelper) try { _timeoutHelper = timeoutHelper; - var responseHelper = new HttpResponseMessageHelper(_httpResponseMessage, _factory, _httpSendCts.Token); + var responseHelper = new HttpResponseMessageHelper(_httpResponseMessage, _factory, _httpSendCts); var replyMessage = await responseHelper.ParseIncomingResponse(timeoutHelper); TryCompleteHttpRequest(_httpRequestMessage); return replyMessage; diff --git a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs index 97a558e793c..9b8a70297e6 100644 --- a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs +++ b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs @@ -17,17 +17,27 @@ namespace System.ServiceModel.Channels { internal class HttpResponseMessageHelper { + private static readonly Action s_cancelCts = state => + { + try + { + ((CancellationTokenSource)state).Cancel(); + } + catch (ObjectDisposedException) + { + // ignore + } + }; + private readonly HttpChannelFactory _factory; private readonly MessageEncoder _encoder; private readonly HttpRequestMessage _httpRequestMessage; private readonly HttpResponseMessage _httpResponseMessage; - private readonly CancellationToken _abortToken; + private readonly CancellationTokenSource _httpSendCts; private string _contentType; private long _contentLength; - private CancellationTokenSource _linkedCts; - private CancellationToken? _cachedCombinedToken; - public HttpResponseMessageHelper(HttpResponseMessage httpResponseMessage, HttpChannelFactory factory, CancellationToken abortToken = default) + public HttpResponseMessageHelper(HttpResponseMessage httpResponseMessage, HttpChannelFactory factory, CancellationTokenSource httpSendCts = null) { Contract.Assert(httpResponseMessage != null); Contract.Assert(httpResponseMessage.RequestMessage != null); @@ -36,36 +46,53 @@ public HttpResponseMessageHelper(HttpResponseMessage httpResponseMessage, HttpCh _httpRequestMessage = httpResponseMessage.RequestMessage; _factory = factory; _encoder = factory.MessageEncoderFactory.Encoder; - _abortToken = abortToken; + _httpSendCts = httpSendCts; } internal async Task ParseIncomingResponse(TimeoutHelper timeoutHelper) { - ValidateAuthentication(); - ValidateResponseStatusCode(); - bool hasContent = await ValidateContentTypeAsync(timeoutHelper); - Message message = null; + // If we have an httpSendCts, register the timeout token to cancel it + // This allows both timeout and abort to cancel stream operations + CancellationTokenRegistration? registration = null; + if (_httpSendCts != null) + { + var timeoutToken = await timeoutHelper.GetCancellationTokenAsync(); + registration = timeoutToken.UnsafeRegister(s_cancelCts, _httpSendCts); + } - if (!hasContent) + try { - if (_encoder.MessageVersion == MessageVersion.None) + ValidateAuthentication(); + ValidateResponseStatusCode(); + bool hasContent = await ValidateContentTypeAsync(timeoutHelper); + Message message = null; + + if (!hasContent) { - message = new NullMessage(); + if (_encoder.MessageVersion == MessageVersion.None) + { + message = new NullMessage(); + } + else + { + return null; + } } else { - return null; + message = await ReadStreamAsMessageAsync(timeoutHelper); } + + var exception = ProcessHttpAddressing(message); + Contract.Assert(exception == null, "ProcessHttpAddressing should not set an exception after parsing a response message."); + + return message; } - else + finally { - message = await ReadStreamAsMessageAsync(timeoutHelper); + // Dispose the registration when we're done + registration?.Dispose(); } - - var exception = ProcessHttpAddressing(message); - Contract.Assert(exception == null, "ProcessHttpAddressing should not set an exception after parsing a response message."); - - return message; } private Exception ProcessHttpAddressing(Message message) @@ -192,7 +219,7 @@ private async Task ReadChunkedBufferedMessageAsync(Task inputSt { try { - return await _encoder.ReadMessageAsync(await inputStreamTask, _factory.BufferManager, _factory.MaxBufferSize, _contentType, await GetCombinedCancellationTokenAsync(timeoutHelper)); + return await _encoder.ReadMessageAsync(await inputStreamTask, _factory.BufferManager, _factory.MaxBufferSize, _contentType, await GetCancellationTokenAsync(timeoutHelper)); } catch (XmlException xmlException) { @@ -216,7 +243,7 @@ private async Task ReadBufferedMessageAsync(Task inputStreamTas byte[] buffer = messageBuffer.Array; int offset = 0; int count = messageBuffer.Count; - var ct = await GetCombinedCancellationTokenAsync(timeoutHelper); + var ct = await GetCancellationTokenAsync(timeoutHelper); while (count > 0) { @@ -277,7 +304,7 @@ private async Task DecodeBufferedMessageAsync(ArraySegment buffer { try { - var ct = await GetCombinedCancellationTokenAsync(timeoutHelper); + var ct = await GetCancellationTokenAsync(timeoutHelper); // if we're chunked, make sure we've consumed the whole body if (_contentLength == -1 && buffer.Count == _factory.MaxReceivedMessageSize) { @@ -308,35 +335,16 @@ private async Task DecodeBufferedMessageAsync(ArraySegment buffer } } - private async Task GetCombinedCancellationTokenAsync(TimeoutHelper timeoutHelper) + private async Task GetCancellationTokenAsync(TimeoutHelper timeoutHelper) { - // If we've already computed the combined token, return it - if (_cachedCombinedToken.HasValue) - { - return _cachedCombinedToken.Value; - } - - var timeoutToken = await timeoutHelper.GetCancellationTokenAsync(); - - // If no abort token is provided or it can't be cancelled, just use the timeout token - if (!_abortToken.CanBeCanceled) - { - _cachedCombinedToken = timeoutToken; - return timeoutToken; - } - - // If the timeout token can't be cancelled, just use the abort token - if (!timeoutToken.CanBeCanceled) + // If no httpSendCts is provided, just use the timeout token + if (_httpSendCts == null) { - _cachedCombinedToken = _abortToken; - return _abortToken; + return await timeoutHelper.GetCancellationTokenAsync(); } - // Both tokens can be cancelled, so create a linked token source - // Store the CTS so we can dispose it later if needed - _linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutToken, _abortToken); - _cachedCombinedToken = _linkedCts.Token; - return _linkedCts.Token; + // Use the _httpSendCts.Token for all operations + return _httpSendCts.Token; } private async Task GetStreamAsync(TimeoutHelper timeoutHelper) @@ -348,7 +356,7 @@ private async Task GetStreamAsync(TimeoutHelper timeoutHelper) { contentStream = await content.ReadAsStreamAsync(); _contentLength = content.Headers.ContentLength.HasValue ? content.Headers.ContentLength.Value : -1; - var cancellationToken = await GetCombinedCancellationTokenAsync(timeoutHelper); + var cancellationToken = await GetCancellationTokenAsync(timeoutHelper); if (_contentLength <= 0) { var preReadBuffer = new byte[1]; From b779be76482f8dc9bb869ccc116d1ff30937e208 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 23:25:58 +0000 Subject: [PATCH 12/13] Improve code clarity with better comments and variable names - Enhanced ObjectDisposedException comment to explain race condition - Renamed 'registration' to 'timeoutCancellationRegistration' for clarity - Improved comment explaining why registration disposal is necessary Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../Channels/HttpResponseMessageHelper.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs index 9b8a70297e6..e2d1c307785 100644 --- a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs +++ b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs @@ -25,7 +25,8 @@ internal class HttpResponseMessageHelper } catch (ObjectDisposedException) { - // ignore + // CancellationTokenSource may have been disposed by the time this callback executes + // due to a race condition between timeout/abort and cleanup } }; @@ -53,11 +54,11 @@ internal async Task ParseIncomingResponse(TimeoutHelper timeoutHelper) { // If we have an httpSendCts, register the timeout token to cancel it // This allows both timeout and abort to cancel stream operations - CancellationTokenRegistration? registration = null; + CancellationTokenRegistration? timeoutCancellationRegistration = null; if (_httpSendCts != null) { var timeoutToken = await timeoutHelper.GetCancellationTokenAsync(); - registration = timeoutToken.UnsafeRegister(s_cancelCts, _httpSendCts); + timeoutCancellationRegistration = timeoutToken.UnsafeRegister(s_cancelCts, _httpSendCts); } try @@ -90,8 +91,8 @@ internal async Task ParseIncomingResponse(TimeoutHelper timeoutHelper) } finally { - // Dispose the registration when we're done - registration?.Dispose(); + // Unregister the timeout callback to prevent memory leaks and avoid canceling after completion + timeoutCancellationRegistration?.Dispose(); } } From 700612dcab0bc49462cf0360990741c0a233ad52 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 23:26:41 +0000 Subject: [PATCH 13/13] Improve comment precision for registration disposal Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com> --- .../System/ServiceModel/Channels/HttpResponseMessageHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs index e2d1c307785..67fb4927043 100644 --- a/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs +++ b/src/System.ServiceModel.Http/src/System/ServiceModel/Channels/HttpResponseMessageHelper.cs @@ -91,7 +91,7 @@ internal async Task ParseIncomingResponse(TimeoutHelper timeoutHelper) } finally { - // Unregister the timeout callback to prevent memory leaks and avoid canceling after completion + // Unregister the timeout callback to prevent memory leaks and avoid invoking the callback after the operation completes timeoutCancellationRegistration?.Dispose(); } }