diff --git a/src/NHibernate.Test/NHSpecificTest/NH3954/Entity.cs b/src/NHibernate.Test/NHSpecificTest/NH3954/Entity.cs new file mode 100644 index 00000000000..d30081241eb --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3954/Entity.cs @@ -0,0 +1,55 @@ +using System; +using System.Collections.Generic; + +namespace NHibernate.Test.NHSpecificTest.NH3954 +{ + class Entity1 + { + } + class Entity1FakeProxy + { + } + class Entity1FakeProxy2 + { + } + + class Entity2 + { + } + class Entity2FakeProxy + { + } + class Entity2FakeProxy2 + { + } + + class Entity3 + { + } + class Entity3FakeProxy + { + } + class Entity3FakeProxy2 + { + } + + class Entity4 + { + } + class Entity4FakeProxy + { + } + class Entity4FakeProxy2 + { + } + + class Entity5 + { + } + class Entity5FakeProxy + { + } + class Entity5FakeProxy2 + { + } +} \ No newline at end of file diff --git a/src/NHibernate.Test/NHSpecificTest/NH3954/EqualsFixture.cs b/src/NHibernate.Test/NHSpecificTest/NH3954/EqualsFixture.cs new file mode 100644 index 00000000000..51cf0323634 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3954/EqualsFixture.cs @@ -0,0 +1,79 @@ +using System; +using System.Reflection; +using NHibernate.Proxy; +using NHibernate.Proxy.DynamicProxy; +using NHibernate.Type; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH3954 +{ + [TestFixture] + public class EqualsFixture + { + private static readonly FieldInfo HashCodeField = + typeof(ProxyCacheEntry).GetField("_hashCode", BindingFlags.Instance | BindingFlags.NonPublic); + + /// + /// Allows to simulate a hashcode collision. Issue would be unpractical to test otherwise. + /// Hashcode collision must be supported for avoiding unexpected and hard to reproduce failures. + /// + private void TweakEntry(ProxyCacheEntry entryToTweak, int hashcode) + { + // Though hashCode is a readonly field, this works at the time of this writing. If it starts breaking and cannot be fixed, + // ignore those tests or throw them away. + HashCodeField.SetValue(entryToTweak, hashcode); + } + + // Non reg test case + [Test] + public void TypeEquality() + { + var entry1 = new ProxyCacheEntry(typeof(Entity1), null); + var entry2 = new ProxyCacheEntry(typeof(Entity1), new System.Type[0]); + Assert.IsTrue(entry1.Equals(entry2)); + Assert.IsTrue(entry2.Equals(entry1)); + } + + [Test] + public void TypeInequality() + { + var entry1 = new ProxyCacheEntry(typeof(Entity1), null); + var entry2 = new ProxyCacheEntry(typeof(Entity2), null); + TweakEntry(entry2, entry1.GetHashCode()); + Assert.IsFalse(entry1.Equals(entry2)); + Assert.IsFalse(entry2.Equals(entry1)); + } + + // Non reg test case + [Test] + public void InterfaceEquality() + { + var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy) }); + // Interfaces order inverted on purpose: must be supported. + var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IProxy), typeof(INHibernateProxy) }); + Assert.IsTrue(entry1.Equals(entry2)); + Assert.IsTrue(entry2.Equals(entry1)); + } + + // Non reg test case + [Test] + public void InterfaceEqualityWithLotOfUnordererdAndDupInterfaces() + { + var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy), typeof(IType), typeof(IDisposable), typeof(IFilter) }); + // Interfaces order inverted and duplicated on purpose: must be supported. + var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IType), typeof(IProxy), typeof(IFilter), typeof(IDisposable), typeof(IType), typeof(IFilter), typeof(INHibernateProxy) }); + Assert.IsTrue(entry1.Equals(entry2)); + Assert.IsTrue(entry2.Equals(entry1)); + } + + [Test] + public void InterfaceInequality() + { + var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy) }); + var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IProxy) }); + TweakEntry(entry2, entry1.GetHashCode()); + Assert.IsFalse(entry1.Equals(entry2)); + Assert.IsFalse(entry2.Equals(entry1)); + } + } +} \ No newline at end of file diff --git a/src/NHibernate.Test/NHSpecificTest/NH3954/ProxyCacheFixture.cs b/src/NHibernate.Test/NHSpecificTest/NH3954/ProxyCacheFixture.cs new file mode 100644 index 00000000000..6b48c931070 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3954/ProxyCacheFixture.cs @@ -0,0 +1,155 @@ +using System.Collections.Concurrent; +using System.Reflection; +using NHibernate.Proxy; +using NHibernate.Proxy.DynamicProxy; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH3954 +{ + [TestFixture, Explicit("Demonstrates bug impact on cache, but which tests will fail is a bit unpredictable")] + public class ProxyCacheFixture + { + private ProxyCache _cache; + private ConcurrentDictionary _internalCache; + private int _hashCode1; + private int _hashCode2; + + private static readonly FieldInfo InternalCacheField = + typeof(ProxyCache).GetField("cache", BindingFlags.Static | BindingFlags.NonPublic); + + [SetUp] + public void SetUp() + { + _cache = new ProxyCache(); + + _internalCache = (ConcurrentDictionary)InternalCacheField.GetValue(null); + + _cache.StoreProxyType(typeof(Entity1FakeProxy), typeof(Entity1)); + _cache.StoreProxyType(typeof(Entity2FakeProxy), typeof(Entity2), typeof(INHibernateProxy)); + _cache.StoreProxyType(typeof(Entity3FakeProxy), typeof(Entity3)); + _cache.StoreProxyType(typeof(Entity4FakeProxy), typeof(Entity4), typeof(IProxy)); + _cache.StoreProxyType(typeof(Entity5FakeProxy), typeof(Entity5), typeof(INHibernateProxy), typeof(IProxy)); + + // Artificially inject other entries with same hashcodes + _hashCode1 = new ProxyCacheEntry(typeof(Entity1), null).GetHashCode(); + Inject(new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy) }), _hashCode1, typeof(Entity1FakeProxy2)); + Inject(new ProxyCacheEntry(typeof(Entity3), null), _hashCode1, typeof(Entity3FakeProxy2)); + + _hashCode2 = new ProxyCacheEntry(typeof(Entity2), new[] { typeof(INHibernateProxy) }).GetHashCode(); + Inject(new ProxyCacheEntry(typeof(Entity2), null), _hashCode2, typeof(Entity2FakeProxy2)); + Inject(new ProxyCacheEntry(typeof(Entity4), new[] { typeof(IProxy) }), _hashCode2, typeof(Entity4FakeProxy2)); + Inject(new ProxyCacheEntry(typeof(Entity5), new[] { typeof(INHibernateProxy), typeof(IProxy) }), _hashCode2, typeof(Entity5FakeProxy2)); + } + + private void Inject(ProxyCacheEntry entryToTweak, int hashcode, System.Type result) + { + TweakEntry(entryToTweak, hashcode); + _internalCache[entryToTweak] = result; + } + + private static readonly FieldInfo HashCodeField = + typeof(ProxyCacheEntry).GetField("_hashCode", BindingFlags.Instance | BindingFlags.NonPublic); + + /// + /// Allows to simulate a hashcode collision. Issue would be unpractical to test otherwise. + /// Hashcode collision must be supported for avoiding unexpected and hard to reproduce failures. + /// + private void TweakEntry(ProxyCacheEntry entryToTweak, int hashcode) + { + // Though hashCode is a readonly field, this works at the time of this writing. If it starts breaking and cannot be fixed, + // ignore those tests or throw them away. + HashCodeField.SetValue(entryToTweak, hashcode); + } + + [Test] + public void ProxyCacheEntity1FakeProxy() + { + var result = _cache.GetProxyType(typeof(Entity1)); + Assert.AreEqual(typeof(Entity1FakeProxy), result); + } + + [Test] + public void ProxyCacheEntity1FakeProxy2() + { + var entry = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy) }); + TweakEntry(entry, _hashCode1); + var result = _internalCache[entry]; + Assert.AreEqual(typeof(Entity1FakeProxy2), result); + } + + [Test] + public void ProxyCacheEntity2FakeProxy() + { + var result = _cache.GetProxyType(typeof(Entity2), typeof(INHibernateProxy)); + Assert.AreEqual(typeof(Entity2FakeProxy), result); + } + + [Test] + public void ProxyCacheEntity2FakeProxy2() + { + var entry = new ProxyCacheEntry(typeof(Entity2), null); + TweakEntry(entry, _hashCode2); + var result = _internalCache[entry]; + Assert.AreEqual(typeof(Entity2FakeProxy2), result); + } + + [Test] + public void ProxyCacheEntity3FakeProxy() + { + var result = _cache.GetProxyType(typeof(Entity3)); + Assert.AreEqual(typeof(Entity3FakeProxy), result); + } + + [Test] + public void ProxyCacheEntity3FakeProxy2() + { + var entry = new ProxyCacheEntry(typeof(Entity3), null); + TweakEntry(entry, _hashCode1); + var result = _internalCache[entry]; + Assert.AreEqual(typeof(Entity3FakeProxy2), result); + } + + [Test] + public void ProxyCacheEntity4FakeProxy() + { + var result = _cache.GetProxyType(typeof(Entity4), typeof(IProxy)); + Assert.AreEqual(typeof(Entity4FakeProxy), result); + } + + [Test] + public void ProxyCacheEntity4FakeProxy2() + { + var entry = new ProxyCacheEntry(typeof(Entity4), new[] { typeof(IProxy) }); + TweakEntry(entry, _hashCode2); + var result = _internalCache[entry]; + Assert.AreEqual(typeof(Entity4FakeProxy2), result); + } + + [Test] + public void ProxyCacheEntity5FakeProxy() + { + var result = _cache.GetProxyType(typeof(Entity5), typeof(IProxy), typeof(INHibernateProxy)); + Assert.AreEqual(typeof(Entity5FakeProxy), result); + } + + [Test] + public void ProxyCacheEntity5FakeProxy2() + { + // Interfaces order inverted on purpose: must be supported. + var entry = new ProxyCacheEntry(typeof(Entity5), new[] { typeof(IProxy), typeof(INHibernateProxy) }); + TweakEntry(entry, _hashCode2); + var result = _internalCache[entry]; + Assert.AreEqual(typeof(Entity5FakeProxy2), result); + } + + [Test] + public void ProxyCacheNone() + { + // Beware not testing the lookup failure of any combination, even tweaked, actually added in cache. + // (Otherwise the test may starts failing unexpectedly sometimes, as the original bug ...) + // This one was not added in anyway. + System.Type result; + Assert.IsFalse(_cache.TryGetProxyType(typeof(Entity2), new[] { typeof(IProxy) }, out result)); + } + } +} \ No newline at end of file diff --git a/src/NHibernate.Test/NHibernate.Test.csproj b/src/NHibernate.Test/NHibernate.Test.csproj index 392cc02a2dc..d99188e2c94 100644 --- a/src/NHibernate.Test/NHibernate.Test.csproj +++ b/src/NHibernate.Test/NHibernate.Test.csproj @@ -736,6 +736,9 @@ + + + diff --git a/src/NHibernate/Proxy/DynamicProxy/ProxyCacheEntry.cs b/src/NHibernate/Proxy/DynamicProxy/ProxyCacheEntry.cs index 4a7aabb47e3..e3e69fe0fa9 100644 --- a/src/NHibernate/Proxy/DynamicProxy/ProxyCacheEntry.cs +++ b/src/NHibernate/Proxy/DynamicProxy/ProxyCacheEntry.cs @@ -11,52 +11,57 @@ namespace NHibernate.Proxy.DynamicProxy { - [Serializable] - public class ProxyCacheEntry + public class ProxyCacheEntry : IEquatable { - private readonly int hashCode; + private readonly int _hashCode; public ProxyCacheEntry(System.Type baseType, System.Type[] interfaces) { if (baseType == null) - { - throw new ArgumentNullException("baseType"); - } + throw new ArgumentNullException(nameof(baseType)); BaseType = baseType; - Interfaces = interfaces ?? new System.Type[0]; + _uniqueInterfaces = new HashSet(interfaces ?? new System.Type[0]); - if (Interfaces.Length == 0) + if (_uniqueInterfaces.Count == 0) { - hashCode = baseType.GetHashCode(); + _hashCode = baseType.GetHashCode(); return; } - - // duplicated type exclusion - var set = new HashSet(Interfaces) { baseType }; - hashCode = 59; - foreach (System.Type type in set) + + var allTypes = new List(_uniqueInterfaces) { baseType }; + _hashCode = 59; + foreach (System.Type type in allTypes) { - hashCode ^= type.GetHashCode(); + // This simple implementation is nonsensitive to list order. If changing it for a sensitive one, + // take care of ordering the list. + _hashCode ^= type.GetHashCode(); } } - public System.Type BaseType { get; private set; } - public System.Type[] Interfaces { get; private set; } + public System.Type BaseType { get; } + public IReadOnlyCollection Interfaces { get { return _uniqueInterfaces; } } + + private HashSet _uniqueInterfaces; public override bool Equals(object obj) { var that = obj as ProxyCacheEntry; - if (ReferenceEquals(null, that)) + return Equals(that); + } + + public bool Equals(ProxyCacheEntry other) + { + if (ReferenceEquals(null, other) || + // hashcode inequality allows an early exit, but their equality is not enough for guaranteeing instances equality. + _hashCode != other._hashCode || + BaseType != other.BaseType) { return false; } - return hashCode == that.GetHashCode(); + return _uniqueInterfaces.SetEquals(other.Interfaces); } - public override int GetHashCode() - { - return hashCode; - } + public override int GetHashCode() => _hashCode; } } \ No newline at end of file