mvvmlight icon indicating copy to clipboard operation
mvvmlight copied to clipboard

SimpleIoc.GetInstanceWithoutCaching returns same instance

Open SteffenRossberg opened this issue 6 years ago • 4 comments

Hello, currently I've seen that GetInstanceWithoutCaching of SimpleIoc returns the same instance like GetInstance in some circumstances. I think GetInstanceWithoutCaching could always return a fresh instance regardless GetInstance was called before or not. See short unit test to enforce this behavior:


        [TestMethod]
        public void GetInstanceWithoutCachingReturnsSameInstanceWhenGetInstanceCalledBefore()
        {
            // Arrange
            var sut = new SimpleIoc();
            sut.Register<IMessenger, Messenger>();

            var notExpected = sut.GetInstance<IMessenger>();

            // Act
            var actual = sut.GetInstanceWithoutCaching<IMessenger>();

            // Assert
            Assert.AreNotEqual(notExpected, actual);
        }

I'm not sure if it is a bug or a feature. Regards, Steffen

SteffenRossberg avatar Dec 25 '17 12:12 SteffenRossberg

According to the documentation you should get a new instance every time. When looking at the implementation it only does a check to see if should cache it when it there is none cached yet , meaning if you do a call anytime to with GetInstance it will create a cache version and that will be used.

Duranom avatar Jan 18 '18 08:01 Duranom

Please give the unit test - posted previously - a chance. In SimpleIoc.cs Line 606 it simply checks if a cached instance exists, if yes then returns it. The cached instance is created by previously call of GetInstance(). If you call GetInstanceWithoutCaching after first GetInstance call, then the simple Ioc will always return same instance. In cases when you always call GetInstanceWithoutCaching() you should get a fresh instance until you call GetInstance(). After one call of GetInstance() then GetInstanceWithoutCaching() does not return fresh instances, only the cached one.

Regards, Steffen

SteffenRossberg avatar Jan 21 '18 12:01 SteffenRossberg

Hello, there is a worst case with this issue. Let's use the class TestClass3 defined in GalaSoft.MvvmLight.Test.Stubs and the unit test :

        [TestMethod]
        public void TestMixImplicitCacheAndNoCache()
        {
            SimpleIoc.Default.Reset();
            SimpleIoc.Default.Register<ITestClass, TestClass1>();
            SimpleIoc.Default.Register<TestClass3>();

            var instance1 = SimpleIoc.Default.GetInstanceWithoutCaching<TestClass3>();
            Assert.IsTrue(SimpleIoc.Default.IsRegistered<TestClass3>());
            Assert.IsFalse(SimpleIoc.Default.ContainsCreated<TestClass3>());

            var instance2 = SimpleIoc.Default.GetInstanceWithoutCaching<ITestClass>();
            Assert.IsTrue(SimpleIoc.Default.IsRegistered<ITestClass>());
            Assert.IsTrue(SimpleIoc.Default.ContainsCreated<ITestClass>());

            var instance3 = SimpleIoc.Default.GetInstanceWithoutCaching<ITestClass>();
            Assert.IsTrue(SimpleIoc.Default.IsRegistered<ITestClass>());
            Assert.IsTrue(SimpleIoc.Default.ContainsCreated<ITestClass>());
            Assert.AreNotSame(instance2, instance3);
        }

The test fails because instance2 and instance3 are the same object.

When creating instance1, SimpleIoc injects a ITestClass service into the TestClass3 constructor. This is made by calling GetService, which results in calling DoGetService with cache = true. So, we implicitly create a cached instance for ITestClass, and cannot obtain a fresh instance anymore.

To avoid this, we can modify the test, line 610, as :

                if (cache
                    && instances != null
                    && instances.ContainsKey(key))
                {
                    return instances[key];
                }

Regards, Axel

Minus-Cortex avatar May 22 '18 09:05 Minus-Cortex

Hello Axel, yes, it seems to fix the problem, when adding cache check to the test in DoGetService (line 610). Regards Steffen

SteffenRossberg avatar May 24 '18 05:05 SteffenRossberg