simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 154 forks source link

Is there a way to check if the Scope was disposed? #976

Closed ilya-malakhovsky closed 9 months ago

ilya-malakhovsky commented 1 year ago

Hi!

I'm using SimpleInjector with Transient and Singleton scopes mostly for my web-application (for historical reasons). Now I want to add some monitoring at the end of each web-request to check if state of objects is correct (basically I'm checking that DBContext doesn't contain any unsaved changes)

For this I've created a Tracker class :

  1. Tracker registered as Scoped in the container
  2. For each creation of IUnitOfWork I've injected resolving of Tracker from the container and adding created IUnitOfWork to tracked list inside the Tracker
  3. On Scope.WhenScopeEnds Tracker checks all registered IUnitOfWorks

(I can't change the Lifetime of IUnitOfWork registration)

Factory method for IUnitOfWork looks like this if simplified:

IUnitOfWork Create()
{
    var unitOfWork = new UnitOfWork(...);

    if (Lifestyle.Scoped.GetCurrentScope(container) != null)
    {
        // Track only if the Scope is valid
        container.GetInstance<Tracker>().Register(unitOfWork);
    }

    return unitOfWork;
}

My problem is that IUnitOfWork can be resolved at any stage of the application (I can't change this) including when there's no web-request yet (I'm checking this as Lifestyle.Scoped.GetCurrentScope(container) != null) and when the request has just ended (checks on HttpApplication.EndRequest).

In the latter case I sometimes get: [ObjectDisposedException] : `Cannot access a disposed object. Object name: 'SimpleInjector.Scope'

Is there a way to know that the Scope was already disposed? In the debug I can see that GetCurrentScope returns me a disposed Scope (I see that it' was disposed via it's internal properties). I would expect it to be null as technically we are out of scope already.

dotnetjunkie commented 1 year ago

Can you supply me with a full stack trace of the exception?

ilya-malakhovsky commented 1 year ago
Unhandled exception during request [500]   [ActivationException] : 
Cannot access a disposed object.  Object name: 'SimpleInjector.Scope'.     at
 SimpleInjector.InstanceProducer.GetInstance()     at
 SimpleInjector.Container.GetInstance[TService]()     at
 MyProject.ServiceConfig.<>c__DisplayClass11_0.<Config>g__Create|1()     at
 lambda_method(Closure )     at
 SimpleInjector.InstanceProducer.GetInstance()     at
 SimpleInjector.Container.GetInstance[TService]()     at
 MyProject.Services.Account.CustomRoleProvider.GetRolesForUser(String username)
 MyProject.MiniProfilerModule.onEndRequest(Object sender, EventArgs e)     at
 System.Web.HttpApplication.SyncEventExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute()     at
 System.Web.HttpApplication.ExecuteStepImpl(IExecutionStep step)     at
 System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously)
[ObjectDisposedException] : `Cannot access a disposed object.  Object name: 'SimpleInjector.Scope'.`     at
 SimpleInjector.Scope.ThrowObjectDisposedException()     at
 SimpleInjector.Scope.GetInstanceInternal(ScopedRegistration registration)     at
 SimpleInjector.Scope.GetInstance[TImplementation](ScopedRegistration registration, Scope scope)     at
 SimpleInjector.Advanced.Internal.LazyScopedRegistration`1.GetInstance(Scope scope)     at
 lambda_method(Closure )     at
 SimpleInjector.InstanceProducer.GetInstance()  

Registration is done like this:

        c.Register<Tracker>(Lifestyle.Scoped);
        c.Register<IUnitOfWork>(Create);

Create is a method from my previous message

dotnetjunkie commented 1 year ago

What you're dealing with is a a combination of corner cases:

Having Simple Injector correctly return null from its GetCurrentScope, however, will unlikely fix your issue, because you want to resolve an instance from a scope. It being null will still not allow you to resolve scoped instances.

Unfortunately, there's little I can do here, because:

  1. This is the way those web frameworks are designed.
  2. I'm not actively developing those integration libraries any longer. I only release new versions with new versions of the Simple Injector core library to keep them compatible, but that's about it.

I would, therefore, suggest a different solution. Here are some suggestions:

I hope this helps

ilya-malakhovsky commented 1 year ago

thanks a lot for your explanation - I'll try to think on alternative ways.

return null from its GetCurrentScope, however, will unlikely fix your issue, because you want to resolve an instance from a scope. It being null will still not allow you to resolve scoped instances

Actually it would help as only Tracker is registered as Scoped and can't be resolved but I'm fine not to track at that moment during this edge case. I.e. UnitOfWork that is required during the work of the profiler is resolved fine since it's registered as Transient.

dotnetjunkie commented 1 year ago

I struggled with the weird request lifestyle model of those web frameworks years ago, which is one of the many reasons I moved to design that I found easier to reason about, which is the use of an ICommandHandler<T> abstraction, which I described here. This allows me to apply cross-cutting concerns like your tracker logic on the boundaries of executed commands.

Changing your application to a design as described in that article might be quite an undertaking, and might not be feasible for you at the moment, but it might give you some inspiration on ways of looking at this from a different angle.