Typed factory should allow disposing multiple times

[jonyadamit]

According to .NET Guidelines a dispose method should tolerate being called multiple times without throwing an exception.
This exception should not be thrown when method equals FactoryMethod.Dispose:

Windsor/src/Castle.Windsor/Facilities/TypedFactory/Internal/TypedFactoryInterceptor.cs

Line 55 in 603d78d

throw new ObjectDisposedException("this", "The factory was disposed and can no longer be used.");

[jonorossi]

Is this causing you a problem in your code? Are you not using Windsor to control the lifetime of your factories?

I suspect it was done this way to make sure Windsor was always cleaning up neatly, no reason the facility should call Dispose more than once.

[jonyadamit]

I'm using transient lifestyle factories in some of my ViewModels. For example, to add a to a collection of child ViewModels.
When a ViewModel is disposed, I am disposing of the factory instead of releasing each item individually.
Once I implemented my Dispose method as the guidelines suggest - my problem was resolved (so I don't really have an issue currently).
But the guidelines are there for a reason.. I believe there should be a good reason not to follow them..

[jonorossi]

Are you calling Resolve to get the factory and then calling Dispose rather than Release?

[jonyadamit]

The factory is resolved as a dependency, I do not care about releasing it.
I am calling Release on individual items when the user removes items from a list for instance.
I am calling Dispose once the entire view is closed/removed.

I don't understand where you are getting at @jonorossi , using the Dispose method is a perfectly valid use case 😄

[jonorossi]

The factory is resolved as a dependency, I do not care about releasing it.
[...]
I am calling Dispose once the entire view is closed/removed.

Are you calling Dispose on the factory? Unless I'm missing something (and that is definitely possible because I don't use this facility and I can't see your code), that class doesn't "own" the factory, Windsor does, it is managing the lifecycle?

[jonyadamit]

"Whatever you explicitly resolve, you should also explicitly release."
It might be that when the factory gets released it will do the same thing, I'm not sure tbh.. But the documentation is very clear about that I should release whatever I resolve.

[jonorossi]

"Whatever you explicitly resolve, you should also explicitly release."
It might be that when the factory gets released it will do the same thing, I'm not sure tbh.. But the documentation is very clear about that I should release whatever I resolve.

I don't follow, you said "The factory is resolved as a dependency" therefore it is not explicitly resolved, so you should not explicitly release.

[jonyadamit]

The Dispose method on the factory is used to release all components it resolved. I explicitly resolve components using this factory.

[ivan-danilov]

OK, a very similar case here: I have a singleton factory, so dependent components explicitly call Release on the factory when their display is closed (WPF thick client app). Now during the application shutdown, Windsor container itself is being disposed of, thus factory gets Dispose call as it should. But due to asynchrony and some complicated interactions - I have no means to control which of the two happens first: disposing of the container/factory or display closing.

Therefore, I'd say that not only the second call to Dispose should be ignored by this interceptor, but also calls to Release method as well after the factory had been disposed.

I can make a PR with the changes and some tests if needed.

[mjacobs80]

I just fell into this and it looks like no progress has been made on this one. Following the documentation about the TypedFactory (https://github.com/castleproject/Windsor/blob/master/docs/typed-factory-facility-interface-based.md) I've created a factory interface that inherits IDisposable.

I have a component that implements IDisposable, it has an interface based TypedFactory injected in the constructor and it's Dispose is called in the components Dispose.

Reviewing the stack trace the exception happens when I've called Dispose on the container itself which looks to match what @ivan-danilov encountered. Basically the order the components are being disposed is not guaranteed hence Dispose being called twice on the factory.

Playing with lifestyles I can confirm that If the component that uses the factory is Transient it doesn't have this issue as a release on a Transient calls it's Dispose (if implemented) immediately. I've not tested what happens if we don't Release explicitly before calling Dispose on the container.

I've got to do some further work checking behaviour under WCF as the component in this case is the service implementation which will be PerWbRequest or PerWcfOperation in many cases. I simply encountered this during unit testing where we've registered using the default lifestyle.

This all said throwing an exception from a Dispose is not expected. As @jonyadamit pointed out, throwing exceptions from Dispose is not in accordance with .NET guidance about allowing dispose to be called multiple times, here is a link about this from plurasight https://www.pluralsight.com/blog/software-development/idisposable-for-dummies-1-why-what

TypedFactory doesn't expose a property to check if it is already disposed, and I don't want to roll my own implementation on the interface as I want interface based typed factories. This appears to leave the only option to remember to put a try catch around it to deal with it robustly in situations where we'd want a component to be a Singleton.

[jonorossi]

Therefore, I'd say that not only the second call to Dispose should be ignored by this interceptor, but also calls to Release method as well after the factory had been disposed.

I can make a PR with the changes and some tests if needed.

@ivan-danilov makes sense. I think I was misunderstanding the situation like @mjacobs80 also has:

Basically the order the components are being disposed is not guaranteed hence Dispose being called twice on the factory.