From c9f75a1f41911e3ff8fb180a8a404d2506cb33db Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Fri, 17 Jul 2020 19:37:20 +0000 Subject: [PATCH] Merged PR 9163: [3.1] Fix HttSys tests The 3.1 PR builds aren't working so I missed some new test failures caused by my prior change. https://dev.azure.com/dnceng/internal/_build/results?buildId=734369&view=ms.vss-test-web.build-test-results-tab HttpSys has some odd behavior when you call SendFileAsync multiple times on an aborted response. The first time throws an OperationCancelledException, but the second time throws an ObjectDisposedException. This only happens if you enable HttpSysOptions.ThrowWriteExceptions and pass in a cancelled token. https://github.com/dotnet/aspnetcore/blob/472fc5058ecbddd4cd546a40628ec5a0545b8125/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs#L577 I'm not aware of any scenarios where SendFileAsync is called multiple times like this, so for now I'm only fixing up the tests. --- .../FunctionalTests/ResponseSendFileTests.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Servers/HttpSys/test/FunctionalTests/ResponseSendFileTests.cs b/src/Servers/HttpSys/test/FunctionalTests/ResponseSendFileTests.cs index 1bda0ddee1..8d54a67a53 100644 --- a/src/Servers/HttpSys/test/FunctionalTests/ResponseSendFileTests.cs +++ b/src/Servers/HttpSys/test/FunctionalTests/ResponseSendFileTests.cs @@ -487,17 +487,21 @@ namespace Microsoft.AspNetCore.Server.HttpSys try { + // Note Response.SendFileAsync uses RequestAborted by default. This can cause the response to be disposed + // before it throws an IOException, but there's a race depending on when the disconnect is noticed. + // Passing our own token to skip that. + using var cts = new CancellationTokenSource(); await Assert.ThrowsAsync(async () => { // It can take several tries before Send notices the disconnect. for (int i = 0; i < Utilities.WriteRetryLimit; i++) { - await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null); + await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null, cts.Token); } }); await Assert.ThrowsAsync(() => - httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null)); + httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null, cts.Token)); testComplete.SetResult(0); } @@ -573,9 +577,13 @@ namespace Microsoft.AspNetCore.Server.HttpSys var testComplete = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); using (Utilities.CreateHttpServer(out var address, async httpContext => { + // Note Response.SendFileAsync uses RequestAborted by default. This can cause the response to be disposed + // before it throws an IOException, but there's a race depending on when the disconnect is noticed. + // Passing our own token to skip that. + using var cts = new CancellationTokenSource(); httpContext.RequestAborted.Register(() => cancellationReceived.SetResult(0)); // First write sends headers - await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null); + await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null, cts.Token); firstSendComplete.SetResult(0); await clientDisconnected.Task; @@ -586,7 +594,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys // It can take several tries before Write notices the disconnect. for (int i = 0; i < Utilities.WriteRetryLimit; i++) { - await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null, CancellationToken.None); + await httpContext.Response.SendFileAsync(AbsoluteFilePath, 0, null, cts.Token); } });