Should PrismBootstrapper class use a static container?

[EMostafaAli]

Hello Prism community,

The current behavior of PrismBootstrapper references a static container defined in ConatinerLocator class. I filed two issues (#2671 & #2673) outlining why this might not be the best approach and @brianlagunas stated that the current design is OK.

With all due respect, I believe using static container is good for PrismApp class but not for PrismBootstrapper, why?

As Brian mentioned in one of his twitch videos, the main reason behind having PrismBootstrapper is for developing plugins for applications (e.g. Visual Studio, AutoCAD, etc.). Now if two different developers create two plugins that use PrismBootstrapper, then when the two plugins are loaded in the host application one of them will override the other (as they use the same container instance). The worst part that the override will be based on which plugin is loaded first as follows:

1. The container created by first plugin will be used by both, see line below

   Prism/src/Prism.Core/Ioc/ContainerLocator.cs

   Line 21 in 7f0b168

   _current ?? (_current = _lazyContainer?.Value);

2. Although the first container will be used, when the second plug register a type (e.g. IRegionManager) it will replace the one defined by first plugin
3. If first plugin register an implantation for a custom interface say ICustomInterface, second plugin will be able to resolve it which is a security concern (remember the second plugin use the same container defined by first plugin)

To be clear, this is not a hypothesis case it is actually happened with AutoCAD plugins and as a plugin developer I don't have control over host applications nor other plugins.

Thanks to prism flexible structure, I am able to override the PrismBootstrapper to use a new instance of the container (rather than the static version) but I believe this should be changed as I can't see a reason for PrismBootstrapper to use a static container (unless it has another use other than plugins).

Looking forward to hear your opinion. Thanks,

[dansiegel]

@EMostafaAli it is important to note regarding the ContainerLocator that this was added to Prism to remove a dependency on the CommonServiceLocator which has been a dependency for WPF applications for as long as I can remember. It's probably also worth noting that, that package is now deprecated as the UnityContainer which it is part of is a dead project.

The CommonServiceLocator itself relies on a Static Singleton to begin with. While in principal I agree with you that it would be much better to have no static singleton instance, ultimately it is a requirement that is not new.

If you would like to dedicate some time to come up with a fully thought out proposal that would work in which any static reference to the container could be eliminated I am certainly willing to entertain it. Both @brianlagunas and I strive to continue to iterate and make Prism an even better framework for building apps, but we do have to work within the confines of the Application Frameworks and with at least some concern for not completely breaking people.

public static class ServiceLocator
{
    private static ServiceLocatorProvider _currentProvider;

    /// <summary>
    /// The current ambient container.
    /// </summary>
    public static IServiceLocator Current
    {
        get 
        {
            if (!IsLocationProviderSet) throw new InvalidOperationException(" ServiceLocationProvider must be set.");

            return _currentProvider(); 
        }
    }

    /// <summary>
    /// Set the delegate that is used to retrieve the current container.
    /// </summary>
    /// <param name="newProvider">Delegate that, when called, will return
    /// the current ambient container.</param>
    public static void SetLocatorProvider(ServiceLocatorProvider newProvider)
    {
        _currentProvider = newProvider;
    }

    public static bool IsLocationProviderSet => _currentProvider != null;
}