Fix: NinjectInstanceProvider.ReleaseInstance() should properly dispose service instances...

[larsw]

Calling KernelContainer.Kernel.Release(instance) should do the trick.

--larsw

[idavis]

The Ninject kernel is managing the lifetime of the components that it creates. Are you seeing behavior where instances are not released properly? IIRC, this should be releasing service instances. What if you are calling release on a singleton service?

-Ian

[larsw]

Hi Ian,

ReleaseInstance() isn't used on singletons (e.g. services w/ InstanceContextMode.Single), see: http://msdn.microsoft.com/en-us/library/system.servicemodel.dispatcher.iinstanceprovider.aspx

That said, PerCall-based services - if implementing IDisposable has to be properly disposed in ReleaseInstance() or you can quickly run into resource starvation situations. Since I'm not that familiar with Ninject (yet), I was hoping to mimic the way we do this with our internal Unity/WCF integration; since Unity supports hierarchical containers, we have an application-wide container and we spin one off for each request. We keep a hash table in the InstanceProvider that keeps track of all the current request containers, and dispose the right one when ReleaseInstance() is reached. We also check if the inbound serviceInstance implements IDisposable, and call Dispose() on it.

So in short; something has to be done with ReleaseInstance() because right now it's can be the cause to real leaks in WCF services. It might be better to actually just do

var disposable = serviceInstance as IDisposable; if (disposable != null) { disposable.Dispose(); }

--larsw

PS: please disregard the funky markup

[idavis]

There is a ninject.extensions.childkernel extension written by Remo that may help what you are trying to do. Ninject does not track transient instances, so it does not dispose of these services automatically. It is assumed that if your object is transient, that you are taking over the lifetime management of the object including disposal. We have gone back and forth on whether to add tracking for transients, but there hasn't been enough push to do so.

[larsw]

Ok, but hear me out: if you leave ReleaseInstance() unimplemented, services implementing IDisposable will not be taken care of (like in the normal instance provider) so there's a bug in there - Ninject or not.

[idavis]

Please don't get me wrong. I see that we have objects not being disposed. I don't see a clean solution yet to prevent us from releasing objects not bound as transient.

[larsw]

Good. I'm thinking about spiking up an alternative with the childkernel extension. IMHO it's cleaner to have a separate child kernel per request to track the transient objects (other goes in the parent container). This is what we do with our WCF/Unity integration.

[idavis]

Is there any way to keep an ActivationBlock in the context of a call?

[larsw]

It can be handled multiple places - but the best and most relevant place is in a IInstanceProvider. It can also be done in an IDispatchMessageInspector implementation.

[remogloor]

The Wcf implementation disposes objects depending on the Ninject scope e.g. InRequestScope bound services are disposed at the end of the request.