SPRNET-1315

Unable to devise a failing test that evidences the issue, but added several tests that demonstrate that its (seemingly)not possible to create the problem described in this issue.  If a failing test is ultimately submitted, we can investigate further.

src/Spring/Spring.Core/Objects/Factory/Support/AbstractObjectFactory.cs

@@ -142,12 +142,6 @@ namespace Spring.Objects.Factory.Support
         /// </summary>
         private readonly Hashtable factoryObjectProductCache = new Hashtable();
 
-
-        /// <summary>
-        /// Collection of singleton-specific locks to support locking singleton-specific critical sections
-        /// </summary>
-        private IDictionary singletonLocks = new Hashtable();
-
         #region Constructor (s) / Destructor
 
         /// <summary>
@@ -190,18 +184,21 @@ namespace Spring.Objects.Factory.Support
             {
                 this.aliasMap = new Hashtable();
                 this.singletonCache = new Hashtable();
+                this.singletonLocks = new Hashtable();
                 this.singletonsInCreation = new Hashtable();
             }
             else
             {
                 this.aliasMap = new CaseInsensitiveHashtable();
                 this.singletonCache = new CaseInsensitiveHashtable();
+                this.singletonLocks = new CaseInsensitiveHashtable();
                 this.singletonsInCreation = new CaseInsensitiveHashtable();
             }
 #else
             IEqualityComparer comparer = (caseSensitive) ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase;
             this.aliasMap = new OrderedDictionary(comparer);
             this.singletonCache = new OrderedDictionary(comparer);
+            this.singletonLocks = new OrderedDictionary(comparer);
             this.singletonsInCreation = new OrderedDictionary(comparer);
 #endif
             this.prototypesInCreation = new LogicalThreadContextSetVariable();
@@ -514,7 +511,7 @@ namespace Spring.Objects.Factory.Support
         {
             AssertUtils.ArgumentHasText(name, "The object name must not be empty.");
             AssertUtils.ArgumentNotNull(singleton, "singleton");
-            lock (singletonCache)
+            lock (GetSingletonLockFor(name))
             {
                 singletonCache[name] = singleton;
                 registeredSingletons.Add(name);
@@ -1160,7 +1157,7 @@ namespace Spring.Objects.Factory.Support
         protected void RemoveSingleton(string name)
         {
             AssertUtils.ArgumentHasText(name, "name");
-            lock (singletonCache)
+            lock (GetSingletonLockFor(name))
             {
                 this.singletonCache.Remove(name);
             }
@@ -1444,28 +1441,16 @@ namespace Spring.Objects.Factory.Support
         /// <seealso cref="Spring.Objects.Factory.Support.AbstractObjectFactory.DestroyObject"/>
         protected virtual void DestroySingleton(string name)
         {
-            object tempObject;
-
-            lock (singletonCache)
+            lock (singletonLocks)
             {
-                tempObject = singletonCache[name];
-                
-                if (tempObject!=null)
-                {
-                    singletonCache.Remove(name);
-                    registeredSingletons.Remove(name);
-                }
+                object tempObject = singletonCache[name];
+                singletonCache.Remove(name);
+                registeredSingletons.Remove(name);
 
-                if (tempObject!=null)
+                object singletonInstance = tempObject;
+                if (singletonInstance != null)
                 {
-                    lock(GetSingletonLockFor(name))
-                    {
-                        object singletonInstance = tempObject;
-                        if (singletonInstance != null)
-                        {
-                            DestroyObject(name, singletonInstance);
-                        }
-                    }
+                    DestroyObject(name, singletonInstance);
                 }
             }
         }
@@ -1583,6 +1568,7 @@ namespace Spring.Objects.Factory.Support
         private bool caseSensitive;
         private IDictionary aliasMap;
         private IDictionary singletonCache;
+        private IDictionary singletonLocks;
 
         /// <summary>
         /// Set of registered singletons, containing the bean names in registration order 
@@ -2140,48 +2126,35 @@ namespace Spring.Objects.Factory.Support
                                                                  RootObjectDefinition objectDefinition,
                                                                  object[] arguments)
         {
-            object sharedInstance;
-
-            lock (singletonCache)
+            lock (GetSingletonLockFor(objectName))
             {
-                sharedInstance = singletonCache[objectName];
-                
+                object sharedInstance = singletonCache[objectName];
                 if (sharedInstance == null)
                 {
-                    lock (GetSingletonLockFor(objectName))
+                    #region Instrumentation
+                    if (log.IsDebugEnabled)
                     {
-                        lock (singletonCache)
-                        {
-                            if (singletonCache.Contains(objectName))
-                            {
-                                return singletonCache[objectName];
-                            }
-                        }
-                        #region Instrumentation
-                        if (log.IsDebugEnabled)
-                        {
-                            log.Debug(string.Format("Creating shared instance of singleton object '{0}'", objectName));
-                        }
-                        #endregion
-
-                        BeforeSingletonCreation(objectName);
-                        try
-                        {
-                            sharedInstance = InstantiateObject(objectName, objectDefinition, arguments, true, false);
-                        }
-                        finally
-                        {
-                            AfterSingletonCreation(objectName);
-                        }
-                        AddSingleton(objectName, sharedInstance);
-
-                        #region Instrumentation
-                        if (log.IsDebugEnabled)
-                        {
-                            log.Debug(string.Format("Cached shared instance of singleton object '{0}'", objectName));
-                        }
-                        #endregion
+                        log.Debug(string.Format("Creating shared instance of singleton object '{0}'", objectName));
                     }
+                    #endregion
+
+                    BeforeSingletonCreation(objectName);
+                    try
+                    {
+                        sharedInstance = InstantiateObject(objectName, objectDefinition, arguments, true, false);
+                    }
+                    finally
+                    {
+                        AfterSingletonCreation(objectName);
+                    }
+                    AddSingleton(objectName, sharedInstance);
+
+                    #region Instrumentation
+                    if (log.IsDebugEnabled)
+                    {
+                        log.Debug(string.Format("Cached shared instance of singleton object '{0}'", objectName));
+                    }
+                    #endregion
                 }
                 return sharedInstance;
             }
@@ -2399,7 +2372,7 @@ namespace Spring.Objects.Factory.Support
         public void RegisterSingleton(string name, object singletonObject)
         {
             AssertUtils.ArgumentHasText(name, "name", "The singleton object cannot be registered under an empty name.");
-            lock (singletonCache)
+            lock (GetSingletonLockFor(name))
             {
                 object oldObject = singletonCache[name];
                 if (oldObject != null)
@@ -2421,7 +2394,7 @@ namespace Spring.Objects.Factory.Support
         public bool ContainsSingleton(string name)
         {
             AssertUtils.ArgumentHasText(name, "name");
-            lock (singletonCache)
+            lock (GetSingletonLockFor(name))
             {
                 return singletonCache.Contains(name);
             }
@@ -2494,7 +2467,7 @@ namespace Spring.Objects.Factory.Support
         /// <returns>The cached object if found, <see langword="null"/> otherwise.</returns>
         public virtual object GetSingleton(string objectName)
         {
-            lock (singletonCache)
+            lock (GetSingletonLockFor(objectName))
             {
                 return singletonCache[objectName];
             }
@@ -2517,7 +2490,6 @@ namespace Spring.Objects.Factory.Support
             return IsAlias(objectName) || ContainsLocalObject(objectName);
         }
 
-
         /// <summary>
         /// Gets the singleton lock for a given object name.
         /// </summary>
@@ -2525,7 +2497,7 @@ namespace Spring.Objects.Factory.Support
         /// <returns>lock object</returns>
         private object GetSingletonLockFor(string objectName)
         {
-            lock (singletonCache)
+            lock (singletonLocks)
             {
                 if (!singletonLocks.Contains(objectName))
                 {

test/Spring/Spring.Core.Tests/Objects/Factory/AbstractObjectFactoryTests.cs

@@ -28,6 +28,7 @@ using Spring.Objects.Factory.Support;
 using Spring.Threading;
 using System.Threading;
 using System.Diagnostics;
+using System.Collections;
 
 #endregion
 
@@ -577,7 +578,6 @@ namespace Spring.Objects.Factory
     }
 #if NET_2_0
     [TestFixture]
-    [Ignore]
     public class SPRNET_1338
     {
         private static AbstractObjectFactory _cachedFactory;
@@ -605,6 +605,7 @@ namespace Spring.Objects.Factory
         }
 
         [Test]
+        [Ignore("Test fails -- waiting for verification re: if bug exists in Java impl")]
         public void CanAvoidLockContentionDuringObjectFactoryDisposal()
         {
             Thread t = new Thread(CreateThreadContentionFromDispose);
@@ -677,5 +678,260 @@ namespace Spring.Objects.Factory
         }
 
     }
+
+    [TestFixture]
+    public class SPRNET_1315
+    {
+        private static AbstractObjectFactory _cachedFactory;
+
+        private static int _childCounter;
+
+        private static int _parentCounter;
+
+        private static ArrayList invocationLog = new ArrayList();
+
+        protected static AbstractObjectFactory ObjectFactory
+        {
+            get { return _cachedFactory; }
+            set { _cachedFactory = value; }
+        }
+
+        [SetUp]
+        public void _Setup()
+        {
+            ObjectFactory = new DefaultListableObjectFactory(true);
+            invocationLog.Clear();
+            _parentCounter = 0;
+            _childCounter = 0;
+        }
+
+        [Test]
+        public void When_ParentAndChildArePrototypes_ConstructorInjection_DoesNotEnforceDestructionOrder()
+        {
+            WireParentAndChildWWithImpliedDependencyByConstructorInjection(false, false);
+
+            Parent theParent = ObjectFactory.GetObject("parent") as Parent;
+            theParent.Dispose();
+
+            Assert.AreEqual(0, _parentCounter, "Should have no remaining parent objects after dispose");
+            Assert.AreEqual(1, _childCounter, "Should have exactly ONE child object");
+            Assert.IsNotNull(theParent.InjectedChild, "Parent's child dependency not set as expected");
+            Assert.AreEqual("Parent Destructor", invocationLog[2], "Parent Destructor wasn't called third!");
+            Assert.AreEqual(3, invocationLog.Count, "Should have no further object lifecycle behavior after parent destruction!");
+        }
+
+        [Test]
+        public void When_ParentAndChildArePrototypes_ConstructorInjection_EnforcesConstructionOrder()
+        {
+            WireParentAndChildWWithImpliedDependencyByConstructorInjection(false, false);
+
+            Parent theParent = ObjectFactory.GetObject("parent") as Parent;
+
+            Assert.AreEqual(1, _parentCounter, "Should have exactly ONE parent object");
+            Assert.AreEqual(1, _childCounter, "Should have exactly ONE child object");
+            Assert.IsNotNull(theParent.InjectedChild, "Parent's child dependency not set as expected");
+            Assert.AreEqual("Child Constructor", invocationLog[0], "Child Constructor wasn't called first!");
+            Assert.AreEqual("Parent Constructor", invocationLog[1], "Parent Constructor wasn't called second!");
+
+        }
+
+        [Test]
+        public void When_ParentAndChildArePrototypes_DependsOn_DoesNotEnforceDestructionOrder()
+        {
+            WireParentAndChildWithDependsOnDeclarationDependency(false, false);
+
+            Parent theParent = ObjectFactory.GetObject("parent") as Parent;
+            theParent.Dispose();
+
+            Assert.AreEqual(0, _parentCounter, "Should have no remaining parent objects after dispose");
+            Assert.AreEqual(1, _childCounter, "Should have exactly ONE child object");
+
+            Assert.AreEqual("Parent Destructor", invocationLog[2], "Parent Destructor wasn't called third!");
+            Assert.AreEqual(3, invocationLog.Count, "Should have no further object lifecycle behavior after parent destruction!");
+        }
+
+        [Test]
+        public void When_ParentAndChildArePrototypes_DependsOn_EnforcesConstructionOrder()
+        {
+            WireParentAndChildWithDependsOnDeclarationDependency(false, false);
+
+            Parent theParent = ObjectFactory.GetObject("parent") as Parent;
+
+            Assert.AreEqual(1, _parentCounter, "Should have exactly ONE parent object");
+            Assert.AreEqual(1, _childCounter, "Should have exactly ONE child object");
+            Assert.AreEqual("Child Constructor", invocationLog[0], "Child Constructor wasn't called first!");
+            Assert.AreEqual("Parent Constructor", invocationLog[1], "Parent Constructor wasn't called second!");
+
+        }
+
+        [Test]
+        public void When_ParentAndChildAreSingletons_ConstructorInjection_EnforcesDestructionOrder()
+        {
+            WireParentAndChildWWithImpliedDependencyByConstructorInjection(true, true);
+
+            //triger the construction of the singletons
+            ObjectFactory.GetObject("parent");
+
+            //trigger the disposal of the singletons
+            ObjectFactory.Dispose();
+
+            Assert.AreEqual(0, _parentCounter, "Should have no remaining parent objects after dispose");
+            Assert.AreEqual(0, _childCounter, "Should have no remaining child objects after dispose");
+            Assert.AreEqual("Parent Destructor", invocationLog[2], "Parent Destructor wasn't called third!");
+            Assert.AreEqual("Child Destructor", invocationLog[3], "Child Destructor wasn't called fourth!");
+            Assert.AreEqual(4, invocationLog.Count, "Should have no further object lifecycle behavior after parent destruction!");
+        }
+
+        [Test]
+        public void When_ParentAndChildAreSingletons_DependsOn_EnforcesDestructionOrder()
+        {
+            WireParentAndChildWithDependsOnDeclarationDependency(true, true);
+
+            //triger the construction of the singletons
+            ObjectFactory.GetObject("parent");
+
+            //make certain they are created successfully
+            Assert.AreEqual(1, _parentCounter, "Should have exactly ONE parent object");
+            Assert.AreEqual(1, _childCounter, "Should have exactly ONE child object");
+
+            //trigger the disposal of the singletons
+            ObjectFactory.Dispose();
+
+            Assert.AreEqual(0, _parentCounter, "Should have no remaining parent objects after dispose");
+            Assert.AreEqual(0, _childCounter, "Should have no remaining child objects after dispose");
+            Assert.AreEqual("Parent Destructor", invocationLog[2], "Parent Destructor wasn't called third!");
+            Assert.AreEqual("Child Destructor", invocationLog[3], "Child Destructor wasn't called fourth!");
+            Assert.AreEqual(4, invocationLog.Count, "Should have no further object lifecycle behavior after child destruction!");
+        }
+
+        [Test]
+        public void When_ParentIsProttpyeAndChildIsSingleton_ConstructorInjection_DoesNotEnforcesDestructionOrder()
+        {
+            WireParentAndChildWWithImpliedDependencyByConstructorInjection(false, true);
+
+            //triger the construction of the singletons
+            ObjectFactory.GetObject("parent");
+
+            //trigger the disposal of the singletons
+            ObjectFactory.Dispose();
+
+            Assert.AreEqual(1, _parentCounter, "Should have ONE remaining parent objects after dispose");
+            Assert.AreEqual(0, _childCounter, "Should have no remaining child objects after dispose");
+            Assert.AreEqual("Child Destructor", invocationLog[2], "Child Destructor wasn't called third!");
+            Assert.AreEqual(3, invocationLog.Count, "Should have no further object lifecycle behavior after child destruction!");
+        }
+
+        [Test]
+        public void When_ParentIsSingletonAndChildIsPrototype_ConstructorInjection_DoesNotEnforcesDestructionOrder()
+        {
+            WireParentAndChildWWithImpliedDependencyByConstructorInjection(true, false);
+
+            //triger the construction of the singletons
+            ObjectFactory.GetObject("parent");
+
+            //trigger the disposal of the singletons
+            ObjectFactory.Dispose();
+
+            Assert.AreEqual(0, _parentCounter, "Should have no remaining parent objects after dispose");
+            Assert.AreEqual(1, _childCounter, "Should have ONE remaining child object after dispose");
+            Assert.AreEqual("Parent Destructor", invocationLog[2], "Child Destructor wasn't called third!");
+            Assert.AreEqual(3, invocationLog.Count, "Should have no further object lifecycle behavior after parent destruction!");
+        }
+
+        [Test]
+        public void When_ParentIsSingletonAndChildIsPrototype_DependsOn_EnforcesDestructionOrder()
+        {
+            WireParentAndChildWithDependsOnDeclarationDependency(true, false);
+
+            //triger the construction of the singletons
+            ObjectFactory.GetObject("parent");
+
+            //make certain they are created successfully
+            Assert.AreEqual(1, _parentCounter, "Should have exactly ONE parent object");
+            Assert.AreEqual(1, _childCounter, "Should have exactly ONE child object");
+
+            //trigger the disposal of the singletons
+            ObjectFactory.Dispose();
+
+            Assert.AreEqual(0, _parentCounter, "Should have no remaining parent objects after dispose");
+            Assert.AreEqual(1, _childCounter, "Should have no remaining child objects after dispose");
+            Assert.AreEqual("Parent Destructor", invocationLog[2], "Parent Destructor wasn't called third!");
+            Assert.AreEqual(3, invocationLog.Count, "Should have no further object lifecycle behavior after parent destruction!");
+        }
+
+        private void WireParentAndChildWithDependsOnDeclarationDependency(bool parentIsSingleton, bool childIsSingleton)
+        {
+            GenericObjectDefinition child = new GenericObjectDefinition();
+            child.ObjectTypeName = typeof(Child).FullName;
+            child.IsSingleton = childIsSingleton;
+            ObjectFactory.RegisterObjectDefinition("child", child);
+
+            GenericObjectDefinition parent = new GenericObjectDefinition();
+            parent.ObjectTypeName = typeof(Parent).FullName;
+            parent.IsSingleton = parentIsSingleton;
+            parent.DependsOn = new string[] { "child" };
+            ObjectFactory.RegisterObjectDefinition("parent", parent);
+        }
+
+        private static void WireParentAndChildWWithImpliedDependencyByConstructorInjection(bool parentIsSingleton, bool childIsSingleton)
+        {
+            GenericObjectDefinition child = new GenericObjectDefinition();
+            child.ObjectTypeName = typeof(Child).FullName;
+            child.IsSingleton = childIsSingleton;
+            ObjectFactory.RegisterObjectDefinition("child", child);
+
+            GenericObjectDefinition parent = new GenericObjectDefinition();
+            parent.ObjectTypeName = typeof(Parent).FullName;
+            parent.IsSingleton = parentIsSingleton;
+            parent.ConstructorArgumentValues.AddIndexedArgumentValue(0, new RuntimeObjectReference("child"));
+            ObjectFactory.RegisterObjectDefinition("parent", parent);
+        }
+
+        public class Parent : IDisposable
+        {
+
+            private Child _child;
+
+            public Parent()
+                : this(null)
+            {
+            }
+
+            public Parent(Child child)
+            {
+                _child = child;
+                _parentCounter++;
+                invocationLog.Add("Parent Constructor");
+            }
+
+            public Child InjectedChild
+            {
+                get { return _child; }
+            }
+
+            public void Dispose()
+            {
+                _parentCounter--;
+                invocationLog.Add("Parent Destructor");
+            }
+        }
+
+        public class Child : IDisposable
+        {
+            public Child()
+            {
+                _childCounter++;
+                invocationLog.Add("Child Constructor");
+            }
+
+            public void Dispose()
+            {
+                _childCounter--;
+                invocationLog.Add("Child Destructor");
+            }
+        }
+
+    }
+
 #endif
 }