From 6bafb7cb6f06a5d5795047d39146df22d24deb50 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Fri, 15 Feb 2019 10:41:21 -0800 Subject: [PATCH] Log before releasing streams and firing tokens (#7583) Might fix: https://github.com/aspnet/AspNetCore-Internal/issues/1659 I think writes complete too fast so we don't observe when the message gets logged --- .../IIS/IIS/src/Core/IISHttpContext.IO.cs | 25 ++++++++----------- .../IIS/IIS/src/Core/IISHttpContext.cs | 2 +- src/Servers/IIS/IIS/src/Core/IISHttpServer.cs | 2 +- .../test/IIS.Tests/ClientDisconnectTests.cs | 4 +-- 4 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs index 119cac01e3..02bd5f8cfd 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs @@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core } catch (ConnectionResetException ex) { - ConnectionReset(); + AbortIO(clientDisconnect: true); error = ex; } catch (Exception ex) @@ -171,7 +171,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core // We want to swallow IO exception and allow app to finish writing catch (ConnectionResetException) { - ConnectionReset(); + AbortIO(clientDisconnect: true); } catch (Exception ex) { @@ -184,11 +184,16 @@ namespace Microsoft.AspNetCore.Server.IIS.Core } } - private bool AbortIO() + internal void AbortIO(bool clientDisconnect) { if (Interlocked.CompareExchange(ref _requestAborted, 1, 0) != 0) { - return false; + return; + } + + if (clientDisconnect) + { + Log.ConnectionDisconnect(_logger, ((IHttpConnectionFeature)this).ConnectionId); } _bodyOutput.Dispose(); @@ -208,8 +213,6 @@ namespace Microsoft.AspNetCore.Server.IIS.Core } }); } - - return true; } public void Abort(Exception reason) @@ -218,15 +221,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core _streams.Abort(reason); NativeMethods.HttpCloseConnection(_pInProcessHandler); - AbortIO(); - } - - internal void ConnectionReset() - { - if (AbortIO()) - { - Log.ConnectionDisconnect(_logger, ((IHttpConnectionFeature)this).ConnectionId); - } + AbortIO(clientDisconnect: false); } } } diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs index fff18b3a4d..fd4da572fa 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs @@ -263,7 +263,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core // don't leak the exception catch (ConnectionResetException) { - ConnectionReset(); + AbortIO(clientDisconnect: true); } } diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpServer.cs b/src/Servers/IIS/IIS/src/Core/IISHttpServer.cs index de292e56c6..d0815ebd08 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpServer.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpServer.cs @@ -164,7 +164,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core try { context = (IISHttpContext)GCHandle.FromIntPtr(pvManagedHttpContext).Target; - context.ConnectionReset(); + context.AbortIO(clientDisconnect: true); } catch (Exception ex) { diff --git a/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs b/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs index 0b4d1af24f..2e77937cb1 100644 --- a/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs +++ b/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs @@ -209,9 +209,9 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { Assert.IsType(exception); } - catch (Exception e) + catch (Exception) { - Logger.LogError(e, "Unexpected exception"); + Logger.LogError(exception, "Unexpected exception type"); throw; } }