volosoft / castle-windsor-ms-adapter

Castle Windsor ASP.NET Core / Microsoft.Extensions.DependencyInjection Adapter
https://www.nuget.org/packages/Castle.Windsor.MsDependencyInjection
MIT License
85 stars 29 forks source link

Memory leaks with ScopedWindsorServiceProvider #26

Closed flexem closed 6 years ago

flexem commented 6 years ago

Hi @hikalkan , I'm using abp v3.1.2 and asp.net zero v4.6.1. with .net framework 4.6.1 and asp.net core 2.0. My application run with large memory(about 2G to 4G), I checked it with the memory profiler tool. and I found the ScopedWindsorServiceProvider have a lot of instances and bytes(see the image). image what's worse, the ScopedWindsorServiceProvider held about 1G instances.

I check the code in this repository, I found the ScopedWindsorServiceProvider implement the IDisposable.but it seems no code invoke the windsor container's Release method to release it.

flexem commented 6 years ago

Here's the root path for ScopedWindsorServiceProvider.because it implement the IDisposed interface. so the windsor container held it. image

flexem commented 6 years ago

I read all the code here,I guess I know how to find the problem. I will check all the call stack for ScopedWindsorServiceProvider. If I have the result,I will show here.

flexem commented 6 years ago

Hi @hikalkan I found the problem is ScopedWindsorServiceProvider is resolved, but did not released by the windsor container. 1. https://github.com/volosoft/castle-windsor-ms-adapter/blob/7b6bf8764a0db16255fb5e0fe823b2493d87ca5e/src/Castle.Windsor.MsDependencyInjection/WindsorRegistrationHelper.cs#L132

Here is the allocation call stack. image 2.

https://github.com/volosoft/castle-windsor-ms-adapter/blob/7b6bf8764a0db16255fb5e0fe823b2493d87ca5e/src/Castle.Windsor.MsDependencyInjection/WindsorServiceScope.cs#L27

Here is the allocation call stack. image

3. https://github.com/volosoft/castle-windsor-ms-adapter/blob/7b6bf8764a0db16255fb5e0fe823b2493d87ca5e/src/Castle.Windsor.MsDependencyInjection/WindsorRegistrationHelper.cs#L21

4. https://github.com/volosoft/castle-windsor-ms-adapter/blob/7b6bf8764a0db16255fb5e0fe823b2493d87ca5e/src/Castle.Windsor.MsDependencyInjection/WindsorServiceProviderFactory.cs#L25

flexem commented 6 years ago

I also changed the test method ResolvingFromScopeAndReleasingShouldWorkForWindsorTransients to test it.

[Fact]
        public void ResolvingFromScopeAndReleasingShouldWorkForWindsorTransients()
        {
            var collection = new ServiceCollection();
            collection.AddScoped<MyTestClass2>();
            collection.AddTransient<MyTestClass3>();

            var serviceProvider = CreateServiceProvider(collection);

            serviceProvider.GetService<IWindsorContainer>().Register(Component.For<MyTestClass1>().LifestyleTransient());

            var scopeFactory = serviceProvider.GetService<IServiceScopeFactory>();
            IServiceProvider scopeServiceProvider;
            using (var scope = scopeFactory.CreateScope())
            {
                scopeServiceProvider = scope.ServiceProvider;
                var testObj1 = scope.ServiceProvider.GetService<MyTestClass1>();
                testObj1.IsDisposed.ShouldBeFalse();
                serviceProvider.GetService<IWindsorContainer>().Release(testObj1);
                testObj1.IsDisposed.ShouldBeTrue();

                _disposeCounter.Get<MyTestClass1>().ShouldBe(1);
                _disposeCounter.Get<MyTestClass2>().ShouldBe(0);
                _disposeCounter.Get<MyTestClass3>().ShouldBe(0);
            }

            Assert.False(_windsorContainer.Kernel.ReleasePolicy.HasTrack(scopeServiceProvider));
            _disposeCounter.Get<MyTestClass2>().ShouldBe(1);
            _disposeCounter.Get<MyTestClass3>().ShouldBe(1);
        }
flexem commented 6 years ago

Hi @hikalkan . I created a pull request to fix this problem.Please have a look.Thank you.