From 572627e88ce4db2ca2b3f18a200d5ff2a73fdead Mon Sep 17 00:00:00 2001 From: David Fowler Date: Tue, 13 Mar 2018 19:37:39 -0700 Subject: [PATCH] Handle posting to the libuv thread after StopAsync (#2388) - Check if the post handle is disposed and noop if it is. We also catch an ObjectDisposedException because it's an inherent race condition. --- .../Internal/LibuvThread.cs | 34 +++++++- .../LibuvThreadTests.cs | 77 +++++++++++++++++++ 2 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 test/Kestrel.Transport.Libuv.Tests/LibuvThreadTests.cs diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvThread.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvThread.cs index f26f656528..bb0f77ee95 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvThread.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvThread.cs @@ -177,11 +177,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal public void Post(Action callback, T state) { + // Handle is closed to don't bother scheduling anything + if (_post.IsClosed) + { + return; + } + var work = new Work { CallbackAdapter = CallbackAdapter.PostCallbackAdapter, Callback = callback, - //TODO: This boxes + // TODO: This boxes State = state }; @@ -189,7 +195,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal { _workAdding.Enqueue(work); } - _post.Send(); + + try + { + _post.Send(); + } + catch (ObjectDisposedException) + { + // There's an inherent race here where we're in the middle of shutdown + } } private void Post(Action callback) @@ -199,6 +213,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal public Task PostAsync(Action callback, T state) { + // Handle is closed to don't bother scheduling anything + if (_post.IsClosed) + { + return Task.CompletedTask; + } + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var work = new Work { @@ -212,7 +232,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal { _workAdding.Enqueue(work); } - _post.Send(); + + try + { + _post.Send(); + } + catch (ObjectDisposedException) + { + // There's an inherent race here where we're in the middle of shutdown + } return tcs.Task; } diff --git a/test/Kestrel.Transport.Libuv.Tests/LibuvThreadTests.cs b/test/Kestrel.Transport.Libuv.Tests/LibuvThreadTests.cs new file mode 100644 index 0000000000..db3b3ebd8f --- /dev/null +++ b/test/Kestrel.Transport.Libuv.Tests/LibuvThreadTests.cs @@ -0,0 +1,77 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests.TestHelpers; +using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests +{ + public class LibuvThreadTests + { + [Fact] + public async Task LibuvThreadDoesNotThrowIfPostingWorkAfterDispose() + { + var mockConnectionHandler = new MockConnectionHandler(); + var mockLibuv = new MockLibuv(); + var transportContext = new TestLibuvTransportContext() { ConnectionHandler = mockConnectionHandler }; + var transport = new LibuvTransport(mockLibuv, transportContext, null); + var thread = new LibuvThread(transport); + var ranOne = false; + var ranTwo = false; + var ranThree = false; + var ranFour = false; + + await thread.StartAsync(); + + await thread.PostAsync(_ => + { + ranOne = true; + }, + null); + + Assert.Equal(1, mockLibuv.PostCount); + + // Shutdown the libuv thread + await thread.StopAsync(TimeSpan.FromSeconds(5)); + + Assert.Equal(2, mockLibuv.PostCount); + + var task = thread.PostAsync(_ => + { + ranTwo = true; + }, + null); + + Assert.Equal(2, mockLibuv.PostCount); + + thread.Post(_ => + { + ranThree = true; + }, + null); + + Assert.Equal(2, mockLibuv.PostCount); + + thread.Schedule(_ => + { + ranFour = true; + }, + (object)null); + + Assert.Equal(2, mockLibuv.PostCount); + + Assert.True(task.IsCompleted); + Assert.True(ranOne); + Assert.False(ranTwo); + Assert.False(ranThree); + Assert.False(ranFour); + } + } +}