CODEWITHSUNDEEP
c#asp.net-web-apidependency-injectioncastle-windsorcastle-windsor-3
RedgoodBreaker
RedgoodBreaker

Reputation: 310

Castle Windsor PerWebRequest object injected in Singleton object without referencing it in field

We have created a singleton object (SsoSettingsProvider ) in which we inject object with lifestyle PerWebRequest (IReservationService in our example it is WCF client). In constructor we use this object to get some data and we place this data in a private field. 

public class SsoSettingsProvider : ISsoSettingsProvider
    {
        readonly LogonSettings _logonSettings;


        public SsoSettingsProvider(IReservationService reservationService)
        {
           _logonSettings = reservationService.GetSSOSettings();
        }        
    }

If we look at possible lifestyle mismatches in Castle Windsor it says:

"Component 'SsoSettingsProvider / ISsoSettingsProvider' with lifestyle Singleton depends on 'late bound IReservationService' with lifestyle PerWebRequest This kind of dependency is usually not desired and may lead to various kinds of bugs."

This info says that there is only possibility, but in this case i think it is not a problem because injected object is not referenced in a field so it can be garbage collected. am i right ?

Upvotes: 2

Views: 740

Answers (1)

Steven
Steven

Reputation: 172646

in this case i think it is not a problem because injected object is not referenced in a field so it can be garbage collected. am i right?

Castle Windsor is warning about Captive Dependencies. The main problem is not so much that instances aren’t garbage collected, but a class will reuse an instance that is not intended for reuse.

Simple example is when you inject a DbContext into a class that is configured as singleton. Although this will result in the DbContext being held alive until its singleton consumer goes out of scope (which typically is when the application ends). A DbContexthowever should not be reused over multiple requests. For one, because it simply isn't thread-safe. On top of that, it gets stale very soon, which causes it to return cached data, instead of re-querying the database.

For this reason we register DbContext typically as Scoped. This does mean however that all its consumers should live at most as long as the DbContext, to prevent it from breaking the application. This is what Castle is warning about.

In your case however you don't store the IReservationService into a private field of SsoSettingsProvider. This would still be a problem, because it would be reasonable to expect that the objects that IReservationService returns do not outlive the IReservationService (otherwise IReservationService would be registered as Singleton). Since from the perspective of SsoSettingsProvider, there is no way it could know whether or not it is safe to store LogonSettings, it is much better to not store it at all.

On top of that, as expressed here, injection constructors should not use their dependencies at all. This leads to slow and unreliable object composition.

So even though you might have analyzed your design and know for sure that this works in your particular case, I would suggest you doing one of the following things:

  • Store IReservationService as private field in SsoSettingsProvider and call GetSSOSettings only when one of SsoSettingsProvider's members is called and prevent storing LogonSettings. This forces you to make either SsoSettingsProvider scoped or IReservationService singleton. Whether or not IReservationService can be singleton is only something you can find out.
  • In case SsoSettingsProvider is only interested in LogonSettings, and LogonSettings is a constant value that won't change after the application started, you should inject LogonSettings directly in SsoSettingsProvider's constructor. This simplifies SsoSettingsProvider and pushes loading the LogonSettings to the Composition Root.

Upvotes: 2

Related Questions