IDisposable ITransientDependency getting disposed again

Topics: Core, Writing modules
Developer
Mar 1, 2012 at 8:21 PM

I see that when disposing the HttpContextScope all dependency instances in the WorkContext are getting disposed too. Now if a dependency (in this case, an ITransientDependency) is IDisposable it will get its Dispose() method called. If that type was properly disposed by the consuming code already (like by wrapping it into a using) this causes it to be disposed again (and BTW if there's an exception thrown and that has a localized message, another exception will be thrown for not being able to get the site's culture, since the WorkContext is null then too; I guess it was also disposed a bit earlier).

I'm curious, what's the best practice here? E.g.:

  1. Store the disposed state in the class and so simply ignore the second Dispose() call.
  2. Make the type a non-ITransientDependency and thus remove from DI.

I would go with 1, but what do you think?

Thanks in advance!

Developer
Mar 1, 2012 at 8:40 PM

I'm not sure that I see the problem, but why wouldn't you use IDependency? That will get disposed after each request.

Developer
Mar 1, 2012 at 8:43 PM
Edited Mar 1, 2012 at 8:43 PM

Ah wait, I think understand now. You're injecting some ITransientDependency which is also IDisposable, into your class. Then you dispose the ITransientDependency from within your class. Correct?

If so, then I think your class should not be responsible for disposing the dependency at all. Instead, it's the factory that provides the service that should manage the lifetime. So you're class should just take the dependency, but not dispose it.

Developer
Mar 1, 2012 at 8:47 PM
Edited Mar 1, 2012 at 8:58 PM

Thanks for your thoughts!

The problem is the second Dispose() call: disposing an already disposed object. Fortunately since it's my class that's in question I can work around like I described.

The class is IDisposable because it handles unmanaged resources that should be cleaned up immediately after they're not needed: that's why I opted not with a simple IDependency. So it's not just a simple class that I want to be garbage-collected early, but a "true" IDisposable.

 

Edit for your second post: yeah, that's right, that's what I'm doing.

I disagree with the responsibility of disposing: IMO IDisposables should be properly disposed when they're not used anymore (that's why the usage of the using block is encouraged), thus freeing up all unmanaged resources the class uses. That's not equal to the life cycle or scope of the object itself, which is managed by the IoC container (and the GC). Since my type here is an ITransientDependency the same instance that's disposed also can't be used elsewhere since all requesting code gets a new instance.

Reading a bit further, on MSDN it's stated that "If an object's Dispose method is called more than once, the object must ignore all calls after the first one.". That's perfectly reasonable and also answers my question: it looks number one is the way to go.

Developer
Mar 1, 2012 at 8:59 PM

I see. I believe that it is a 'best practice' to have an IsDisposed flag on your IDisposable types, so option 1 would probably make the most sense. I think I've learned that from Effective C# by Bill Wagner

Developer
Mar 1, 2012 at 9:05 PM
Edited Mar 1, 2012 at 9:06 PM
Piedone wrote:

Thanks for your thoughts!

The problem is the second Dispose() call: disposing an already disposed object. Fortunately since it's my class that's in question I can work around like I described.

The class is IDisposable because it handles unmanaged resources that should be cleaned up immediately after they're not needed: that's why I opted not with a simple IDependency. So it's not just a simple class that I want to be garbage-collected early, but a "true" IDisposable.

 

Edit for your second post: yeah, that's right, that's what I'm doing.

I disagree with the responsibility of disposing: IMO IDisposables should be properly disposed when they're not used anymore (that's why the usage of the using block is encouraged), thus freeing up all unmanaged resources the class uses. That's not equal to the life cycle or scope of the object itself, which is managed by the IoC container (and the GC). Since my type here is an ITransientDependency the same instance that's disposed also can't be used elsewhere since all requesting code gets a new instance.

Reading a bit further, on MSDN it's stated that "If an object's Dispose method is called more than once, the object must ignore all calls after the first one.". That's perfectly reasonable and also answers my question: it looks number one is the way to go.


Agreed. The IoC container may give you transient objects, but it couldn't possibly know when to dispose them, since they're transient. So you're right about disposing it as soon as you're done with it.

Yes, option 1 would be the way to go.

Developer
Mar 1, 2012 at 9:11 PM

Thanks for your time!