diff --git a/src/Spring/Spring.Core/Objects/Factory/Support/AbstractObjectFactory.cs b/src/Spring/Spring.Core/Objects/Factory/Support/AbstractObjectFactory.cs
index 53a01151..62ca9716 100644
--- a/src/Spring/Spring.Core/Objects/Factory/Support/AbstractObjectFactory.cs
+++ b/src/Spring/Spring.Core/Objects/Factory/Support/AbstractObjectFactory.cs
@@ -142,12 +142,6 @@ namespace Spring.Objects.Factory.Support
///
private readonly Hashtable factoryObjectProductCache = new Hashtable();
-
- ///
- /// Collection of singleton-specific locks to support locking singleton-specific critical sections
- ///
- private IDictionary singletonLocks = new Hashtable();
-
#region Constructor (s) / Destructor
///
@@ -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
///
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;
///
/// 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
/// The cached object if found, otherwise.
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);
}
-
///
/// Gets the singleton lock for a given object name.
///
@@ -2525,7 +2497,7 @@ namespace Spring.Objects.Factory.Support
/// lock object
private object GetSingletonLockFor(string objectName)
{
- lock (singletonCache)
+ lock (singletonLocks)
{
if (!singletonLocks.Contains(objectName))
{
diff --git a/test/Spring/Spring.Core.Tests/Objects/Factory/AbstractObjectFactoryTests.cs b/test/Spring/Spring.Core.Tests/Objects/Factory/AbstractObjectFactoryTests.cs
index 944e20fa..064aa6dd 100644
--- a/test/Spring/Spring.Core.Tests/Objects/Factory/AbstractObjectFactoryTests.cs
+++ b/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
}
\ No newline at end of file