Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Sami Al Khatib
    @alsami
    Yeah you could do that as described
    Albert Hives
    @ahives
    thanks
    brsmith080
    @brsmith080
    hey, Im looking for a bit of guidance, I have some work happening on a new aspnet core project that is using autofac for DI needs. Some folks are making argument that every class needs to implement IDisposable, and every class needs to explicitly dispose constructor injected services. I disagree. Anyway, I'm looking for pointers for one side of the argument or the other. Anyone have input?
    Michael Wegge
    @Firedragonweb
    On the contrary, that would be fatal. First of, if lifetime scopes are being used correctly, autofac handles calling dispose at the end of the lifetime scope.
    Second, components naturally have different lifetimes. Imagine a component that is very short lived (instance per dependency) which has a dependency on a service that lifes for the entire application lifetime (Single Instance). The short-lived component would, at the end of its lifetime, dispose the long lived one and effectively render it unusable for any component coming after it
    Sami Al Khatib
    @alsami
    @brsmith080 @Firedragonweb totally agree. Autofac takes care of disposing objects with the right lifetime. For instance something registered as InstancePerLifetimeScope in ASP.NET Core is created and disposed on each request. It's definitely not necessary to implement IDisposible on each class. Even without Autofac it might not be required unless the class is really holding lots of state. This accepted answer here is quite good:
    https://stackoverflow.com/questions/538060/proper-use-of-the-idisposable-interface
    brsmith080
    @brsmith080
    awesome
    I like the stackoverflow answer too
    Ive brought along the autofac documentation around disposal and the .net core DI scoped lifetimes documentation to the discussion
    but those dont seem to be getting everyone on the same page
    got any advice for me? :D
    Sami Al Khatib
    @alsami
    Where did you do that?
    brsmith080
    @brsmith080
    the discussion?
    Sami Al Khatib
    @alsami
    Important part of the accepted answer is, that unmanaged code must be cleaned-up manually
    brsmith080
    @brsmith080
    right
    Sami Al Khatib
    @alsami
    everything else the garbage-collector can take care of
    brsmith080
    @brsmith080
    the only piece here that would have unmanaged resources is the ef context, but it implements IDisposable. It's provided by autofac and reigstered as a per scope lifetime. So autofac should be disposing it, correct?
    Sami Al Khatib
    @alsami
    I am not sure if in this case it would be the framework that disposes the object but yes, it is being disposed after each request has ended.
    The documentation here is also quite clear about it and you can even call some clean-up functionality yourself:
    https://autofaccn.readthedocs.io/en/latest/lifetime/disposal.html#specified-disposal
    brsmith080
    @brsmith080
    right, that's what I brought along to the discussion
    brsmith080
    @brsmith080
    thanks for the input!
    Michael Wegge
    @Firedragonweb
    There are more reasons to have an object disposed - for example if it subscribes an event of a longer lived service. If that is not a weak event, the object would memory leak and not be garbage collected if it does not unsubscribe the event
    Without autofac (and I think this is where they are coming from) you would need a "dispose-chain" up to the top level object of the unit of work. However, since autofac tracks disposables, this is no longer necessary
    brsmith080
    @brsmith080
    @Firedragonweb so which way do you lean? we need to dispose explicitly sometimes, or does autofac cover it?
    brsmith080
    @brsmith080
    just to be on the same page, no events are in play. as far as I can tell the only real disposable service in play is an EF db context
    and the other side of this internal argument wants every service that depends on the db context to explicitly dispose it
    and as a ripple effect, everything upstream to explicitly dispose those services
    I think that's bad design, with an IOC container or not - but Ive been wrong before. Im looking for supporting evidence on either side of the argument
    Sami Al Khatib
    @alsami

    and the other side of this internal argument wants every service that depends on the db context to explicitly dispose it

    That definitely doesn't sound like the right idea. Even if there were events running in the background, you can let autofac dispose the object and just implement IDisposable in that specific to stop the listener.

    Michael Wegge
    @Firedragonweb
    Exactly: To let every component that depends on the ef context dispose it could on the contrary very well lead to a wrong behavior.
    Imagine two components, depending on the db context, that want to update a table in the db on dispose (regardless of that being arguably rather bad style). If every one of those dispose the context after the update, there cannot be a right disposal order for them. One of them will always run into an exception due to the context having already been disposed
    brsmith080
    @brsmith080
    right, that makes sense to me
    and I've brought most of those things to my side of the argument already
    the whole thing is pretty frustrating :D
    Alex Meyer-Gleaves
    @alexmg
    Here is a simple rule you can apply. If the class creates an IDisposable instance directly for internal purposes then it should dispose it because the container does not known about it. If the class has an IDisposable dependency injected into it then the container will dispose it for you when the time is right. IMHO, lifetime management is probably the single greatest feature the container provides. https://nblumhardt.com/2011/01/an-autofac-lifetime-primer/
    brsmith080
    @brsmith080
    @alexmg I agree with lifetime management being a great feature
    and that's one of the reasons I landed on using autofac forever and ever ago :D
    so far Im losing this argument, somehow
    my next step here is to write some unit tests to show the problems that could occur
    Michael Wegge
    @Firedragonweb
    What are the counterarguments?
    Nicholas Blumhardt
    @nblumhardt
    Some folks are making argument that every class needs to implement IDisposable, and every class needs to explicitly dispose constructor injected services. I disagree. Anyway, I'm looking for pointers for one side of the argument or the other.
    @brsmith080 I think the simple "you created it, you dispose it? you didn't created it? don't dispose it" rule is the only really straightforward way to manage IDisposable in .NET. A class that accepts a dependency has no way at all of knowing whether it uniquely uses that dependency, or if it is shared with other consumers - disposing it then becomes arbitrary and prone to causing ObjectDisposedExceptions for other consumers.
    If we try an alternative, hypothetical "you used it? you dispose it" rule, we end up in a position where it's never possible for two consumers to use the same disposable resource - it really doesn't make sense.
    The Autofac behavior just encodes the former rule: if Autofac creates something, Autofac (alone) should dispose it.
    Good luck! :-)
    brsmith080
    @brsmith080
    @Firedragonweb honestly, Im not entirely sure - seems to be some fear of memory leak problems
    and maybe some confusion of what the disposable pattern's purpose is
    someone said something that made me think they expect dispose to do the GC's job in terms of cleanup for a given thing
    but, Im hoping I just misunderstood him
    @nblumhardt yeah, that's been my argument so far too, and autofac doing a good job of cleaning up after itself is one of the features that brought me to it forever ago