Skip to content

Commit

Permalink
clear scope.disposables collection when disposing container to preven…
Browse files Browse the repository at this point in the history
…t ConcurrentBag memory leak

ConcurrentBag is using thread locals variables to store it's content. in some cases when the Bag contains elements
that have circular references (reference an instance of this Bag) the whole graph of objects stay in memory
if the Bag is not cleared before all references are released
  • Loading branch information
Yuriy Lazebnyk authored and jeremydmiller committed Feb 20, 2021
1 parent 38e83d2 commit 3fa6197
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 22 deletions.
17 changes: 17 additions & 0 deletions src/Lamar.Testing/IoC/Acceptance/disposing_container.cs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,23 @@ public void should_dispose_lambda_instance_with_transient_scope_parent_is_scoped
disposableDependent.WasDisposed.ShouldBeTrue();
disposableDependent.ChildDisposable.WasDisposed.ShouldBeTrue();
}

[Theory]
[InlineData(DisposalLock.Ignore)]
[InlineData(DisposalLock.Unlocked)]
public void should_clear_disposables_collection_when_disposed(DisposalLock disposalLock)
{
var container = new Container(x =>
{
x.For<Disposable>().Use<Disposable>();
});

var service = container.GetInstance<Disposable>();
container.DisposalLock = disposalLock;
container.Dispose();

container.Disposables.ShouldBeEmpty();
}
}

public class DisposableDependent : IDisposable
Expand Down
34 changes: 12 additions & 22 deletions src/Lamar/IoC/Scope.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.CodeDom.Compiler;
using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
Expand All @@ -10,7 +9,6 @@
using Lamar.IoC.Diagnostics;
using Lamar.IoC.Frames;
using Lamar.IoC.Instances;
using Lamar.Scanning;
using LamarCodeGeneration;
using LamarCodeGeneration.Model;
using LamarCodeGeneration.Util;
Expand Down Expand Up @@ -45,8 +43,6 @@ public Scope(IServiceCollection services, PerfTimer timer = null)
Bootstrapping.MarkStart("Lamar Scope Creation");
}



Root = this;

Bootstrapping.MarkStart("Build ServiceGraph");
Expand All @@ -63,16 +59,12 @@ public Scope(IServiceCollection services, PerfTimer timer = null)
{
Bootstrapping.MarkFinished("Lamar Scope Creation");
}


}

protected Scope(){}

public Scope Root { get; protected set; }



public Scope(ServiceGraph serviceGraph, Scope root)
{
ServiceGraph = serviceGraph;
Expand Down Expand Up @@ -103,18 +95,21 @@ protected void assertNotDisposed()

internal readonly Dictionary<int, object> Services = new Dictionary<int, object>();



public virtual void Dispose()
{
if (DisposalLock == DisposalLock.Ignore) return;

if (DisposalLock == DisposalLock.ThrowOnDispose) throw new InvalidOperationException("This Container has DisposalLock = DisposalLock.ThrowOnDispose and cannot be disposed until the lock is cleared");

if (_hasDisposed) return;
_hasDisposed = true;

foreach (var disposable in Disposables.Distinct())
var distinctDisposables = Disposables.Distinct().ToArray();
// clear disposables bag to prevent memory leak. current implementation of ConcurrentBag is using thread local storage and in some cases
// e.g. an object from Disposables collection is referencing this Scope instance the whole graph can stay in memory after it was disposed
while (Disposables.TryTake(out _)) { }

if (DisposalLock == DisposalLock.Ignore) return;

foreach (var disposable in distinctDisposables)
{
disposable.SafeDispose();
}
Expand Down Expand Up @@ -209,8 +204,7 @@ public object QuickBuild(Type objectType)
var constructorInstance = new ConstructorInstance(objectType, objectType, ServiceLifetime.Transient);
var ctor = constructorInstance.DetermineConstructor(ServiceGraph, out var message);
var setters = constructorInstance.FindSetters(ServiceGraph);



if (ctor == null) throw new InvalidOperationException(message);

var dependencies = ctor.GetParameters().Select(x =>
Expand Down Expand Up @@ -270,7 +264,6 @@ public IEnumerable GetAllInstances(Type serviceType)
}



public string WhatDoIHave(Type serviceType = null, Assembly assembly = null, string @namespace = null,
string typeName = null)
{
Expand All @@ -285,7 +278,7 @@ public string WhatDoIHave(Type serviceType = null, Assembly assembly = null, str
TypeName = typeName
});
}

public string HowDoIBuild(Type serviceType = null, Assembly assembly = null, string @namespace = null,
string typeName = null)
{
Expand Down Expand Up @@ -342,13 +335,10 @@ public string WhatDidIScan()
}
}


public IServiceVariableSource CreateServiceVariableSource()
{
return new ServiceVariableSource(ServiceGraph);
}



public string GenerateCodeWithInlineServices(GeneratedAssembly assembly)
{
Expand Down Expand Up @@ -398,7 +388,7 @@ public void TryAddDisposable(object @object)
Disposables.Add(disposable);
}
}

public object AddDisposable(object @object)
{
Disposables.Add((IDisposable) @object);
Expand All @@ -409,7 +399,7 @@ public Func<string, T> FactoryByNameFor<T>()
{
return GetInstance<T>;
}

public Func<T> FactoryFor<T>()
{
return GetInstance<T>;
Expand Down

0 comments on commit 3fa6197

Please sign in to comment.