diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvConnectionContext.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvConnectionContext.cs index 6627376b7d..4f76cc097f 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvConnectionContext.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvConnectionContext.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal public IPEndPoint RemoteEndPoint { get; set; } public IPEndPoint LocalEndPoint { get; set; } - public PipeFactory PipeFactory => ListenerContext.Thread.PipelineFactory; + public PipeFactory PipeFactory => ListenerContext.Thread.PipeFactory; public IScheduler InputWriterScheduler => ListenerContext.Thread; public IScheduler OutputReaderScheduler => ListenerContext.Thread; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvThread.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvThread.cs index 393aaa6af5..11b279c66f 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvThread.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/Internal/LibuvThread.cs @@ -69,7 +69,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal #endif QueueCloseHandle = PostCloseHandle; QueueCloseAsyncHandle = EnqueueCloseHandle; - PipelineFactory = new PipeFactory(); + PipeFactory = new PipeFactory(); WriteReqPool = new WriteReqPool(this, _log); ConnectionManager = new LibuvConnectionManager(this); } @@ -83,7 +83,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal public UvLoopHandle Loop { get { return _loop; } } - public PipeFactory PipelineFactory { get; } + public PipeFactory PipeFactory { get; } public LibuvConnectionManager ConnectionManager { get; } @@ -140,7 +140,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal } catch (ObjectDisposedException) { - // Until we rework this logic, ODEs are bound to happen sometimes. if (!await WaitAsync(_threadTcs.Task, stepTimeout).ConfigureAwait(false)) { _log.LogCritical($"{nameof(LibuvThread)}.{nameof(StopAsync)} failed to terminate libuv thread."); @@ -157,34 +156,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal private async Task DisposeConnectionsAsync() { - try + // Close and wait for all connections + if (!await ConnectionManager.WalkConnectionsAndCloseAsync(_shutdownTimeout).ConfigureAwait(false)) { - // Close and wait for all connections - if (!await ConnectionManager.WalkConnectionsAndCloseAsync(_shutdownTimeout).ConfigureAwait(false)) - { - _log.NotAllConnectionsClosedGracefully(); + _log.NotAllConnectionsClosedGracefully(); - if (!await ConnectionManager.WalkConnectionsAndAbortAsync(TimeSpan.FromSeconds(1)).ConfigureAwait(false)) - { - _log.NotAllConnectionsAborted(); - } + if (!await ConnectionManager.WalkConnectionsAndAbortAsync(TimeSpan.FromSeconds(1)).ConfigureAwait(false)) + { + _log.NotAllConnectionsAborted(); } - - var result = await WaitAsync(PostAsync(state => - { - var listener = state; - listener.WriteReqPool.Dispose(); - }, - this), _shutdownTimeout).ConfigureAwait(false); - - if (!result) - { - _log.LogError(0, null, "Disposing write requests failed"); - } - } - finally - { - PipelineFactory.Dispose(); } } @@ -334,8 +314,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal } finally { - _threadTcs.SetResult(null); + PipeFactory.Dispose(); + WriteReqPool.Dispose(); thisHandle.Free(); + _threadTcs.SetResult(null); } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs index 8803ded61a..9c53d9e18d 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs @@ -172,6 +172,78 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } + [Fact] + public async Task ServerShutsDownGracefullyWhenMaxRequestBufferSizeExceeded() + { + // Parameters + var data = new byte[_dataLength]; + var bytesWrittenTimeout = TimeSpan.FromMilliseconds(100); + var bytesWrittenPollingInterval = TimeSpan.FromMilliseconds(bytesWrittenTimeout.TotalMilliseconds / 10); + var maxSendSize = 4096; + + var startReadingRequestBody = new ManualResetEvent(false); + var clientFinishedSendingRequestBody = new ManualResetEvent(false); + var lastBytesWritten = DateTime.MaxValue; + + using (var host = StartWebHost(16 * 1024, data, false, startReadingRequestBody, clientFinishedSendingRequestBody)) + { + var port = host.GetPort(); + using (var socket = CreateSocket(port)) + using (var stream = new NetworkStream(socket)) + { + await WritePostRequestHeaders(stream, data.Length); + + var bytesWritten = 0; + + Func sendFunc = async () => + { + while (bytesWritten < data.Length) + { + var size = Math.Min(data.Length - bytesWritten, maxSendSize); + await stream.WriteAsync(data, bytesWritten, size); + bytesWritten += size; + lastBytesWritten = DateTime.Now; + } + + clientFinishedSendingRequestBody.Set(); + }; + + var gnore = sendFunc(); + + // The minimum is (maxRequestBufferSize - maxSendSize + 1), since if bytesWritten is + // (maxRequestBufferSize - maxSendSize) or smaller, the client should be able to + // complete another send. + var minimumExpectedBytesWritten = (16 * 1024) - maxSendSize + 1; + + // The maximum is harder to determine, since there can be OS-level buffers in both the client + // and server, which allow the client to send more than maxRequestBufferSize before getting + // paused. We assume the combined buffers are smaller than the difference between + // data.Length and maxRequestBufferSize. + var maximumExpectedBytesWritten = data.Length - 1; + + // Block until the send task has gone a while without writing bytes AND + // the bytes written exceeds the minimum expected. This indicates the server buffer + // is full. + // + // If the send task is paused before the expected number of bytes have been + // written, keep waiting since the pause may have been caused by something else + // like a slow machine. + while ((DateTime.Now - lastBytesWritten) < bytesWrittenTimeout || + bytesWritten < minimumExpectedBytesWritten) + { + await Task.Delay(bytesWrittenPollingInterval); + } + + // Verify the number of bytes written before the client was paused. + Assert.InRange(bytesWritten, minimumExpectedBytesWritten, maximumExpectedBytesWritten); + + // Dispose host prior to closing connection to verify the server doesn't throw during shutdown + // if a connection no longer has alloc and read callbacks configured. + host.Dispose(); + } + } + } + private static IWebHost StartWebHost(long? maxRequestBufferSize, byte[] expectedBody, bool useConnectionAdapter, ManualResetEvent startReadingRequestBody, ManualResetEvent clientFinishedSendingRequestBody) {