2

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 ?

RedgoodBreaker
  • 310
  • 1
  • 10
  • Please read [this article](http://blog.ploeh.dk/2014/06/02/captive-dependency/) to understand the problem. – Steven Aug 29 '17 at 10:45
  • thanks for the article. I am aware of this behaviour but it is not the same scenario. Mapping the article to my situation we should have reference to `IReservationService` in a field of `SsoSettingsProvider`. in my example it is used once. – RedgoodBreaker Aug 30 '17 at 20:05
  • 1
    And [this article](http://blog.ploeh.dk/2011/03/03/InjectionConstructorsshouldbesimple/) applies to your situation as well. – Steven Aug 30 '17 at 20:15
  • Good point about SRP. Do You think `LogonSettings` should be injected using `.UsingFactoryMethod()` in DI configuration ? – RedgoodBreaker Sep 01 '17 at 12:20

1 Answers1

2

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.
Steven
  • 166,672
  • 24
  • 332
  • 435
  • in my case i have constant value. So i think the best option is usung `.UsingFactoryMethod()` – RedgoodBreaker Sep 04 '17 at 08:42
  • If it is a value that doesn't change after the application has started, register it as singleton instance using `.Instance`. That would be much easier than using `.UsingFactoryMethod`. – Steven Sep 04 '17 at 08:48
  • but i have to call `SsoSettingsProvider` WCF Client to get `LogonSettings` from remote service. So i must create instance o `SsoSettingsProvider ` – RedgoodBreaker Sep 04 '17 at 09:09
  • You can do it before making registrations to your container. – Steven Sep 04 '17 at 09:12