From 28e3c8331b9a5235a397927b41904b5cf0a15b99 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Mon, 21 Nov 2016 14:02:56 -0800 Subject: [PATCH] Dispose Hubs from ActivatorUtilities --- SignalR.sln | 7 ++ .../HubEndPoint.cs | 49 +++++--- .../HubEndpointTests.cs | 107 ++++++++++++++++++ .../Microsoft.AspNetCore.SignalR.Tests.xproj | 20 ++++ .../Properties/AssemblyInfo.cs | 19 ++++ .../project.json | 31 +++++ 6 files changed, 215 insertions(+), 18 deletions(-) create mode 100644 test/Microsoft.AspNetCore.SignalR.Tests/HubEndpointTests.cs create mode 100644 test/Microsoft.AspNetCore.SignalR.Tests/Microsoft.AspNetCore.SignalR.Tests.xproj create mode 100644 test/Microsoft.AspNetCore.SignalR.Tests/Properties/AssemblyInfo.cs create mode 100644 test/Microsoft.AspNetCore.SignalR.Tests/project.json diff --git a/SignalR.sln b/SignalR.sln index 3991e3144b..5e114980b9 100644 --- a/SignalR.sln +++ b/SignalR.sln @@ -43,6 +43,8 @@ Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "SocialWeather", "samples\So EndProject Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNetCore.SignalR.Test.Server", "test\Microsoft.AspNetCore.SignalR.Test.Server\Microsoft.AspNetCore.SignalR.Test.Server.xproj", "{A0BF246B-FE7D-4E12-99BF-FFDC131B85D8}" EndProject +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNetCore.SignalR.Tests", "test\Microsoft.AspNetCore.SignalR.Tests\Microsoft.AspNetCore.SignalR.Tests.xproj", "{1CE2B3BE-056C-41E3-A5F5-6A1EF1D288BA}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -105,6 +107,10 @@ Global {A0BF246B-FE7D-4E12-99BF-FFDC131B85D8}.Debug|Any CPU.Build.0 = Debug|Any CPU {A0BF246B-FE7D-4E12-99BF-FFDC131B85D8}.Release|Any CPU.ActiveCfg = Release|Any CPU {A0BF246B-FE7D-4E12-99BF-FFDC131B85D8}.Release|Any CPU.Build.0 = Release|Any CPU + {1CE2B3BE-056C-41E3-A5F5-6A1EF1D288BA}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {1CE2B3BE-056C-41E3-A5F5-6A1EF1D288BA}.Debug|Any CPU.Build.0 = Debug|Any CPU + {1CE2B3BE-056C-41E3-A5F5-6A1EF1D288BA}.Release|Any CPU.ActiveCfg = Release|Any CPU + {1CE2B3BE-056C-41E3-A5F5-6A1EF1D288BA}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -124,5 +130,6 @@ Global {300979F6-A02E-407A-B8DF-F6200806C18D} = {C4BC9889-B49F-41B6-806B-F84941B2549B} {8D789F94-CB74-45FD-ACE7-92AF6E55042E} = {C4BC9889-B49F-41B6-806B-F84941B2549B} {A0BF246B-FE7D-4E12-99BF-FFDC131B85D8} = {6A35B453-52EC-48AF-89CA-D4A69800F131} + {1CE2B3BE-056C-41E3-A5F5-6A1EF1D288BA} = {6A35B453-52EC-48AF-89CA-D4A69800F131} EndGlobalSection EndGlobal diff --git a/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs b/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs index 8d1b24262f..76b9934af2 100644 --- a/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs +++ b/src/Microsoft.AspNetCore.SignalR/HubEndPoint.cs @@ -63,9 +63,15 @@ namespace Microsoft.AspNetCore.SignalR using (var scope = _serviceScopeFactory.CreateScope()) { - var hub = scope.ServiceProvider.GetService() ?? Activator.CreateInstance(); - InitializeHub(connection, hub); + bool created; + var hub = CreateHub(scope.ServiceProvider, connection, out created); + await hub.OnConnectedAsync(); + + if (created) + { + hub.Dispose(); + } } await DispatchMessagesAsync(connection); @@ -74,9 +80,15 @@ namespace Microsoft.AspNetCore.SignalR { using (var scope = _serviceScopeFactory.CreateScope()) { - var hub = scope.ServiceProvider.GetService() ?? Activator.CreateInstance(); - InitializeHub(connection, hub); + bool created; + var hub = CreateHub(scope.ServiceProvider, connection, out created); + await hub.OnDisconnectedAsync(); + + if (created) + { + hub.Dispose(); + } } await _lifetimeManager.OnDisconnectedAsync(connection); @@ -132,11 +144,22 @@ namespace Microsoft.AspNetCore.SignalR } } - private void InitializeHub(Connection connection, THub hub) + private THub CreateHub(IServiceProvider provider, Connection connection, out bool created) { + var hub = provider.GetService(); + created = false; + + if (hub == null) + { + hub = ActivatorUtilities.CreateInstance(provider); + created = true; + } + hub.Clients = _hubContext.Clients; hub.Context = new HubCallerContext(connection); hub.Groups = new GroupManager(connection, _lifetimeManager); + + return hub; } private void DiscoverHubMethods() @@ -169,17 +192,8 @@ namespace Microsoft.AspNetCore.SignalR using (var scope = _serviceScopeFactory.CreateScope()) { - var hub = scope.ServiceProvider.GetService(); - - bool created = false; - - if (hub == null) - { - hub = Activator.CreateInstance(); - created = true; - } - - InitializeHub(connection, hub); + bool created; + var hub = CreateHub(scope.ServiceProvider, connection, out created); try { @@ -212,8 +226,7 @@ namespace Microsoft.AspNetCore.SignalR if (created) { - // Dispose the object if it's disposable and we created it - (hub as IDisposable)?.Dispose(); + hub.Dispose(); } } diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/HubEndpointTests.cs b/test/Microsoft.AspNetCore.SignalR.Tests/HubEndpointTests.cs new file mode 100644 index 0000000000..ac4c913a99 --- /dev/null +++ b/test/Microsoft.AspNetCore.SignalR.Tests/HubEndpointTests.cs @@ -0,0 +1,107 @@ +// 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.IO.Pipelines; +using System.Linq; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Hosting.Internal; +using Microsoft.AspNetCore.Sockets; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace Microsoft.AspNetCore.SignalR.Tests +{ + public class HubEndpointTests + { + [Fact] + public async Task HubsAreDisposed() + { + var trackDispose = new TrackDispose(); + var serviceProvider = CreateServiceProvider(s => s.AddSingleton(trackDispose)); + + var endPoint = serviceProvider.GetService>(); + + using (var connectionWrapper = new ConnectionWrapper()) + { + var endPointTask = endPoint.OnConnectedAsync(connectionWrapper.Connection); + + await connectionWrapper.HttpConnection.Input.ReadingStarted; + + // kill the connection + connectionWrapper.Connection.Channel.Dispose(); + + await endPointTask; + + Assert.Equal(2, trackDispose.DisposeCount); + } + } + + private class TestHub : Hub + { + private TrackDispose _trackDispose; + + public TestHub(TrackDispose trackDispose) + { + _trackDispose = trackDispose; + } + + protected override void Dispose(bool dispose) + { + if (dispose) + { + _trackDispose.DisposeCount++; + } + } + } + + private class TrackDispose + { + public int DisposeCount = 0; + } + + private IServiceProvider CreateServiceProvider(Action addServices = null) + { + var services = new ServiceCollection(); + services.AddOptions() + .AddLogging() + .AddSignalR(); + + addServices?.Invoke(services); + + return services.BuildServiceProvider(); + } + + private class ConnectionWrapper : IDisposable + { + private PipelineFactory _factory; + private HttpConnection _httpConnection; + + public Connection Connection; + public HttpConnection HttpConnection => (HttpConnection)Connection.Channel; + + public ConnectionWrapper(string format = "json") + { + _factory = new PipelineFactory(); + _httpConnection = new HttpConnection(_factory); + + var lifetime = new ApplicationLifetime(new Logger(new LoggerFactory()), Enumerable.Empty()); + var connectionManager = new ConnectionManager(lifetime); + + Connection = connectionManager.AddNewConnection(_httpConnection).Connection; + Connection.Metadata["formatType"] = format; + Connection.User = new ClaimsPrincipal(new ClaimsIdentity()); + } + + public void Dispose() + { + Connection.Channel.Dispose(); + _httpConnection.Dispose(); + _factory.Dispose(); + } + } + } +} diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/Microsoft.AspNetCore.SignalR.Tests.xproj b/test/Microsoft.AspNetCore.SignalR.Tests/Microsoft.AspNetCore.SignalR.Tests.xproj new file mode 100644 index 0000000000..8c3454e3e2 --- /dev/null +++ b/test/Microsoft.AspNetCore.SignalR.Tests/Microsoft.AspNetCore.SignalR.Tests.xproj @@ -0,0 +1,20 @@ + + + + 14.0 + $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) + + + + 1ce2b3be-056c-41e3-a5f5-6a1ef1d288ba + .\obj + .\bin\ + + + 2.0 + + + + + + \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/Properties/AssemblyInfo.cs b/test/Microsoft.AspNetCore.SignalR.Tests/Properties/AssemblyInfo.cs new file mode 100644 index 0000000000..a41208291b --- /dev/null +++ b/test/Microsoft.AspNetCore.SignalR.Tests/Properties/AssemblyInfo.cs @@ -0,0 +1,19 @@ +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +// General Information about an assembly is controlled through the following +// set of attributes. Change these attribute values to modify the information +// associated with an assembly. +[assembly: AssemblyConfiguration("")] +[assembly: AssemblyCompany("")] +[assembly: AssemblyProduct("Microsoft.AspNetCore.SignalR.Tests")] +[assembly: AssemblyTrademark("")] + +// Setting ComVisible to false makes the types in this assembly not visible +// to COM components. If you need to access a type in this assembly from +// COM, set the ComVisible attribute to true on that type. +[assembly: ComVisible(false)] + +// The following GUID is for the ID of the typelib if this project is exposed to COM +[assembly: Guid("1ce2b3be-056c-41e3-a5f5-6a1ef1d288ba")] \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/project.json b/test/Microsoft.AspNetCore.SignalR.Tests/project.json new file mode 100644 index 0000000000..6a9a7addc3 --- /dev/null +++ b/test/Microsoft.AspNetCore.SignalR.Tests/project.json @@ -0,0 +1,31 @@ +{ + "buildOptions": { + "warningsAsErrors": true + }, + + "dependencies": { + "dotnet-test-xunit": "2.2.0-*", + "Microsoft.AspNetCore.Hosting": "1.2.0-*", + "Microsoft.AspNetCore.Sockets": { + "target": "project" + }, + "Microsoft.AspNetCore.SignalR": { + "target": "project" + }, + "Microsoft.Extensions.DependencyInjection": "1.2.0-*", + "Microsoft.Extensions.Logging": "1.2.0-*", + "xunit": "2.2.0-*" + }, + "frameworks": { + "netcoreapp1.1": { + "dependencies": { + "Microsoft.NETCore.App": { + "version": "1.1.0-*", + "type": "platform" + } + } + }, + "net451": { } + }, + "testRunner": "xunit" +}