The general rule of resources is that:
He who owns the resource is responsible of disposing of it.
This means that if a class owns a resource, it should either dispose of it in the same method that it created it in (in which case the disposable is called an ephemeral disposable), or if that’s not possible, this generally means that the owning class must implement IDisposable, so it can dispose of the resource in its Dispose method.
But it is important to note that, in general, a class should only own a resource if it is responsible of its creation. When a resource is injected, however, it means that this resource already existed before the consumer did. The consumer didn't create the resource and should, therefore, not dispose of it. Although you can pass on the ownership of the resource to the consumer (and communicate this in the class’s documentation that ownership is passed on), in general you should not pass on the ownership, because this complicates your code and makes the application fragile.
Although the strategy of transferring ownership of objects might make sense in some cases, for instance for types that are part of a reusable API (like System.IO.StreamReader), it not a good idea when dealing with components that are part of your object graph. I’ll explain why below.
So even if your Controller depends on services that need to be disposed of, your controller should not dispose of them:
- Because the consumer did not create such dependency, it has no idea what the expected lifetime of its dependency is. It could be very well that the dependency should outlive the consumer. Letting the consumer dispose of that dependency will, in that case, cause a bug in your application, because the next controller will get an already disposed of dependency, and this will cause an
ObjectDisposedException to be thrown.
- Even if the lifestyle of the dependency equals that of the consumer, disposing of it is still problematic, because this prevents you from easily replacing that component for one that might have a longer lifetime in the future. Once you replace that component for a longer-lived component, you will have to go through all it consumers, possibly causing sweeping changes throughout the application. In other words, you will be violating the Open/closed principle by doing that–it should be possible to add or replace functionality without making sweeping changes instead.
- If your consumer is able to dispose of its dependencies, this means that you have to implement
IDisposable on their abstractions. This means such abstraction is leaking implementation details–that’s a violation of the Dependency Inversion Principle. You are leaking implementation details when implementing IDisposable on an abstraction, because it is very unlikely that every implementation of that abstraction needs deterministic disposal and so you defined the abstraction with a certain implementation in mind. The consumer should not have to know anything about the implementation, whether it needs deterministic disposal or not.
- Letting that abstraction implement
IDisposable also causes you to violate the Interface Segregation Principle, because the abstraction now contains an extra method (i.e. Dispose), that not all consumers need to call. They might not need to call it, because –as I mentioned– the resource might outlive the consumer. Letting it implement IDisposable in that case is dangerous, because anyone can call Dispose on it causing the application to break. If you are more strict about testing, this also means you will have to test a consumer for not calling the Dispose method. This will cause extra test code. This is code that needs to be written and maintained.
Instead, you should only place IDisposable on the implementation. This frees any consumer of the abstraction from the doubts whether it should or shouldn't call Dispose (because there is no Dispose method to call on the abstraction).
Because only the component implements IDisposable and only your Composition Root creates components, it is the Composition Root that is responsible of its disposal. In case your DI container creates this resource, it should also dispose of it. DI Containers like Autofac will actually do this for you. You can easily verify this. In case you wire your object graphs without the use of a DI Container (a.k.a. Pure DI), it means that you will have to dispose of those objects in your Composition Root yourself.
Considering the object graph given in your question, a simplistic code example that demonstrates both resolve (i.e. composing) and release (i.e. dispose of) would like like this:
// Create disposable component and hold reference to it
var session = new Session();
// create the complete object graph including the disposable
var controller =
new Controller(
new Manager(
new Repository(
session)));
// use the object graph
controller.TellYoMamaJoke();
// Clean up resources
session.Dispose();
Of course this example ignores complicating factors such as implementing deterministic cleanup, integration with application frameworks, and the use of DI Containers, but hopefully this code helps painting a mental model.
Note that this design change makes your code simpler. Implementing IDisposable on the abstraction and letting consumers dispose of their dependencies will cause IDisposable to spread out through your system like a virus and pollutes your code base. It spreads out, because for any abstraction, you can always think of an implementation that needs to clean up its resources, so you will have to implement IDisposable on every abstraction. This means that every implementation that takes one or more dependencies also has to implement IDisposable as well, and this cascades up the object graph. This adds a lot of code and unnecessary complexity to each class in your system.
