Suggested Change for SimpleInjectorInstanceProvider

Sep 17, 2013 at 9:01 PM
Hi,
Could I suggest you change the method with the signature
public void ReleaseInstance(InstanceContext instanceContext, object instance)

to this..

public void ReleaseInstance(InstanceContext instanceContext, object instance)
    {
        var scope = this.container.GetCurrentWcfOperationScope();

        if (scope != null)
        {
            scope.Dispose();
        }
    }
the method GetCurrentWcfOperationScope() has this line..
      // CurrentScope can be null, when there is currently no scope.
            return manager.CurrentScope;
I am using the simplinjector with Azure web and worker roles and this method is throwing a null reference exception because CurrentScope null. Unless I am missing something.

Thanks,
O
Coordinator
Sep 18, 2013 at 7:05 AM
If the scope is null, there's a serious problem. This might be caused by Azure releasing the instance on a different thread. Skipping the scope.Dispose only makes things worse, since the scope is stored in a thread-local field and not disposing it means it keeps hanging around and will create a memory leak.

In this case you'd be better of using the LifetimeScope lifestyle, and wrapping your operation methods with a using (container.BeginLifetimeScope()) { ... } call, assuming that your web service methods don't return Task<T> or are async methods.
Sep 18, 2013 at 8:21 AM
HI,
thanks for the response. Yeah, my operation methods do return Tasks and make use of the async/await pattern. You say that "Skipping the scope.Dispose only makes things worse", but as I understand it, at this point, there is nothing to dispose of. ReleaseInstance is only being called once. It is consistent on every call at the moment. I'm not sure what (if any) the alternative is. I am about to load test the system with the suggested change, so any memory leak should manifest itself. I'll report the results...

Regards,
O
Coordinator
Sep 18, 2013 at 8:55 AM
Not disposing the scope, means all instances that are cached in the lifestyle. This means that those instances will stay alive forever. Unfortunately in your case, it is impossible to dispose, since you're disposing on a different thread, and because of this, the ReleaseInstance method can't get to the WcfOperationScopeManager can't get to the right WcfOperationScope.

Since you are using async/await, using a lifetime scope won't help either, since it also depends running on a single thread.

There's currently no built-in solution in Simple Injector that allows working with scoped lifestyles in a async/await driven application. There's already a discussion about this here in the context of Web API, and this holds for you as well. Blueling proposed a new LifetimeScope here lifestyle that works correctly in .NET 4.5 with async/await. Your best bet is to try out his code, and work with that in your scenario untill this issue is resolved. Simple Injector will support your scenario in the future.

I hope this helps.
Marked as answer by dot_NET_Junkie on 3/2/2014 at 11:05 AM
Sep 18, 2013 at 9:53 AM
Brilliant. Thanks. Your candour is appreciated.....


Developer
Oct 13, 2013 at 9:22 PM
Edited Oct 13, 2013 at 10:06 PM
It is not directly realated to Oferns's suggestion but to the scope-disposal policy in general.

For the ExecutionContextScopeLifestyle impl that Steve mentioned I made a tiny change to the behavior of EndLifetimeScope() in LifetimeScopeManager.cs.

Suggested change:
internal void EndLifetimeScope(LifetimeScope scope)
{
    if (object.ReferenceEquals(this.CurrentScope, scope))
    {
        var parent = scope.ParentScope;

        while (parent != null && parent.IsDisposed)
        {
            parent = parent.ParentScope;
        }

        this.threadLocalScopes.Value = parent;
    }
}
Original version:
internal void EndLifetimeScope(LifetimeScope scope)
{
    this.threadLocalScopes.Value = scope.ParentScope;
}
The question is how nested scopes should behave if they are not disposed in the reverse order in which they were created. Here is some code which demonstrates a problem with the current impl (creating an orphaned object in an already disposed scope):
var c = new Container();
c.RegisterLifetimeScope<IFace1, ImplA>();
var scopeA = c.BeginLifetimeScope();
var scopeB = c.BeginLifetimeScope();
var scopeC = c.BeginLifetimeScope();
c.GetInstance<IFace1>();    // created in scopeC
scopeA.Dispose();           // now disposing the most outer scope
try
{
    c.GetInstance<IFace1>(); // oops ... no scope
}
catch (Exception e)
{
    Trace.WriteLine(e.Message);
}
scopeC.Dispose();   // now magical reactivation of scopeB by Dispose()
c.GetInstance<IFace1>();    // created in scopeB
scopeB.Dispose();   // .. now activating already disposed scopeA?!?
c.GetInstance<IFace1>();    // created in disposed scopeA?
scopeA.Dispose();   // ops - effectively a nop! ERROR: orphaned object!!