Skip to content

Fix ProxyFactory cache #1454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 5, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions src/NHibernate.Test/NHSpecificTest/NH3954/EqualsFixture.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Reflection;
using System.Runtime.Serialization;
using NHibernate.Proxy;
using NHibernate.Proxy.DynamicProxy;
using NHibernate.Type;
Expand Down Expand Up @@ -48,9 +49,9 @@ public void TypeInequality()
[Test]
public void InterfaceEquality()
{
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy) });
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(ISerializable) });
// Interfaces order inverted on purpose: must be supported.
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IProxy), typeof(INHibernateProxy) });
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(ISerializable), typeof(INHibernateProxy) });
Assert.IsTrue(entry1.Equals(entry2));
Assert.IsTrue(entry2.Equals(entry1));
}
Expand All @@ -59,21 +60,21 @@ public void InterfaceEquality()
[Test]
public void InterfaceEqualityWithLotOfUnordererdAndDupInterfaces()
{
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(IProxy), typeof(IType), typeof(IDisposable), typeof(IFilter) });
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(ISerializable), 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) });
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(IType), typeof(ISerializable), 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) });
var entry1 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(INHibernateProxy), typeof(ISerializable) });
var entry2 = new ProxyCacheEntry(typeof(Entity1), new[] { typeof(ISerializable) });
TweakEntry(entry2, entry1.GetHashCode());
Assert.IsFalse(entry1.Equals(entry2));
Assert.IsFalse(entry2.Equals(entry1));
}
}
}
}
34 changes: 18 additions & 16 deletions src/NHibernate.Test/NHSpecificTest/NH3954/ProxyCacheFixture.cs
Original file line number Diff line number Diff line change
@@ -1,34 +1,36 @@
using System.Collections.Concurrent;
using System;
using System.Collections.Concurrent;
using System.Reflection;
using System.Runtime.Serialization;
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")]
[TestFixture, Explicit("Demonstrates bug impact on cache, but which tests will fail is a bit unpredictable"), Obsolete]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to mark it as obsolete as ProxyCache class now is marked obsolete

public class ProxyCacheFixture
{
private ProxyCache _cache;
private ConcurrentDictionary<ProxyCacheEntry, System.Type> _internalCache;
private ConcurrentDictionary<ProxyCacheEntry, TypeInfo> _internalCache;
private int _hashCode1;
private int _hashCode2;

private static readonly FieldInfo InternalCacheField =
typeof(ProxyCache).GetField("cache", BindingFlags.Static | BindingFlags.NonPublic);
typeof(ProxyFactory).GetField("_cache", BindingFlags.Static | BindingFlags.NonPublic);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field has moved to ProxyFactory


[SetUp]
public void SetUp()
{
_cache = new ProxyCache();

_internalCache = (ConcurrentDictionary<ProxyCacheEntry, System.Type>)InternalCacheField.GetValue(null);
_internalCache = (ConcurrentDictionary<ProxyCacheEntry, TypeInfo>)InternalCacheField.GetValue(null);

_cache.StoreProxyType(typeof(Entity1FakeProxy).GetTypeInfo(), typeof(Entity1));
_cache.StoreProxyType(typeof(Entity2FakeProxy).GetTypeInfo(), typeof(Entity2), typeof(INHibernateProxy));
_cache.StoreProxyType(typeof(Entity3FakeProxy).GetTypeInfo(), typeof(Entity3));
_cache.StoreProxyType(typeof(Entity4FakeProxy).GetTypeInfo(), typeof(Entity4), typeof(IProxy));
_cache.StoreProxyType(typeof(Entity5FakeProxy).GetTypeInfo(), typeof(Entity5), typeof(INHibernateProxy), typeof(IProxy));
_cache.StoreProxyType(typeof(Entity4FakeProxy).GetTypeInfo(), typeof(Entity4), typeof(ISerializable));
_cache.StoreProxyType(typeof(Entity5FakeProxy).GetTypeInfo(), typeof(Entity5), typeof(INHibernateProxy), typeof(ISerializable));

// Artificially inject other entries with same hashcodes
_hashCode1 = new ProxyCacheEntry(typeof(Entity1), null).GetHashCode();
Expand All @@ -37,14 +39,14 @@ public void SetUp()

_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));
Inject(new ProxyCacheEntry(typeof(Entity4), new[] { typeof(ISerializable) }), _hashCode2, typeof(Entity4FakeProxy2));
Inject(new ProxyCacheEntry(typeof(Entity5), new[] { typeof(INHibernateProxy), typeof(ISerializable) }), _hashCode2, typeof(Entity5FakeProxy2));
}

private void Inject(ProxyCacheEntry entryToTweak, int hashcode, System.Type result)
{
TweakEntry(entryToTweak, hashcode);
_internalCache[entryToTweak] = result;
_internalCache[entryToTweak] = result.GetTypeInfo();
}

private static readonly FieldInfo HashCodeField =
Expand Down Expand Up @@ -112,14 +114,14 @@ public void ProxyCacheEntity3FakeProxy2()
[Test]
public void ProxyCacheEntity4FakeProxy()
{
var result = _cache.GetProxyType(typeof(Entity4), typeof(IProxy));
var result = _cache.GetProxyType(typeof(Entity4), typeof(ISerializable));
Assert.AreEqual(typeof(Entity4FakeProxy), result);
}

[Test]
public void ProxyCacheEntity4FakeProxy2()
{
var entry = new ProxyCacheEntry(typeof(Entity4), new[] { typeof(IProxy) });
var entry = new ProxyCacheEntry(typeof(Entity4), new[] { typeof(ISerializable) });
TweakEntry(entry, _hashCode2);
var result = _internalCache[entry];
Assert.AreEqual(typeof(Entity4FakeProxy2), result);
Expand All @@ -128,15 +130,15 @@ public void ProxyCacheEntity4FakeProxy2()
[Test]
public void ProxyCacheEntity5FakeProxy()
{
var result = _cache.GetProxyType(typeof(Entity5), typeof(IProxy), typeof(INHibernateProxy));
var result = _cache.GetProxyType(typeof(Entity5), typeof(ISerializable), 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) });
var entry = new ProxyCacheEntry(typeof(Entity5), new[] { typeof(ISerializable), typeof(INHibernateProxy) });
TweakEntry(entry, _hashCode2);
var result = _internalCache[entry];
Assert.AreEqual(typeof(Entity5FakeProxy2), result);
Expand All @@ -149,7 +151,7 @@ public void ProxyCacheNone()
// (Otherwise the test may starts failing unexpectedly sometimes, as the original bug ...)
// This one was not added in anyway.
TypeInfo result;
Assert.IsFalse(_cache.TryGetProxyType(typeof(Entity2), new[] { typeof(IProxy) }, out result));
Assert.IsFalse(_cache.TryGetProxyType(typeof(Entity2), new[] { typeof(ISerializable) }, out result));
}
}
}
}
2 changes: 1 addition & 1 deletion src/NHibernate/Async/Engine/Cascade.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@


using System.Collections;

using System.Collections.Generic;
using NHibernate.Collection;
using NHibernate.Event;
using NHibernate.Persister.Collection;
Expand Down
22 changes: 5 additions & 17 deletions src/NHibernate/Proxy/DynamicProxy/DefaultMethodEmitter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,12 @@
using System.Diagnostics;
using System.Reflection;
using System.Reflection.Emit;
using NHibernate.Linq;
using NHibernate.Util;

namespace NHibernate.Proxy.DynamicProxy
{
internal class DefaultMethodEmitter : IMethodBodyEmitter
{
private static readonly MethodInfo getInterceptor;

private static readonly MethodInfo handlerMethod = ReflectHelper.GetMethod<IInterceptor>(
i => i.Intercept(null));
private static readonly MethodInfo getArguments = typeof(InvocationInfo).GetMethod("get_Arguments");
Expand All @@ -33,17 +30,8 @@ internal class DefaultMethodEmitter : IMethodBodyEmitter
typeof (object[])
});

private static readonly PropertyInfo interceptorProperty = typeof (IProxy).GetProperty("Interceptor");

private static readonly ConstructorInfo notImplementedConstructor = typeof(NotImplementedException).GetConstructor(new System.Type[0]);

private readonly IArgumentHandler _argumentHandler;

static DefaultMethodEmitter()
{
getInterceptor = interceptorProperty.GetGetMethod();
}

public DefaultMethodEmitter() : this(new DefaultArgumentHandler()) {}

public DefaultMethodEmitter(IArgumentHandler argumentHandler)
Expand All @@ -60,12 +48,12 @@ public void EmitMethodBody(MethodBuilder proxyMethod, MethodBuilder callbackMeth
ParameterInfo[] parameters = method.GetParameters();
IL.DeclareLocal(typeof (object[]));
IL.DeclareLocal(typeof (InvocationInfo));
IL.DeclareLocal(typeof(System.Type[]));
IL.DeclareLocal(typeof (System.Type[]));

IL.Emit(OpCodes.Ldarg_0);
IL.Emit(OpCodes.Callvirt, getInterceptor);
IL.Emit(OpCodes.Ldfld, field);

// if (interceptor == null)
// if (this.__interceptor == null)
// return base.method(...);

Label skipBaseCall = IL.DefineLabel();
Expand All @@ -90,9 +78,9 @@ public void EmitMethodBody(MethodBuilder proxyMethod, MethodBuilder callbackMeth
IL.Emit(OpCodes.Newobj, infoConstructor);
IL.Emit(OpCodes.Stloc_1);

// this.Interceptor.Intercept(info);
// this.__interceptor.Intercept(info);
IL.Emit(OpCodes.Ldarg_0);
IL.Emit(OpCodes.Callvirt, getInterceptor);
IL.Emit(OpCodes.Ldfld, field);
IL.Emit(OpCodes.Ldloc_1);
IL.Emit(OpCodes.Callvirt, handlerMethod);

Expand Down
5 changes: 4 additions & 1 deletion src/NHibernate/Proxy/DynamicProxy/HashSetExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
using System;
using System.Collections.Generic;

namespace NHibernate.Proxy.DynamicProxy
{
[Obsolete("This class is not used anymore and will be removed in a next major version")]
public static class HashSetExtensions
{
[Obsolete("This method is not used anymore and will be removed in a next major version")]
public static HashSet<T> Merge<T>(this HashSet<T> source, IEnumerable<T> toMerge)
{
foreach (T item in toMerge)
Expand All @@ -13,4 +16,4 @@ public static HashSet<T> Merge<T>(this HashSet<T> source, IEnumerable<T> toMerge
return source;
}
}
}
}
5 changes: 4 additions & 1 deletion src/NHibernate/Proxy/DynamicProxy/IProxyCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@

namespace NHibernate.Proxy.DynamicProxy
{
//Since v5.1
[Obsolete("This class is not used anymore and will be removed in a next major version")]
public interface IProxyCache
{
bool Contains(System.Type baseType, params System.Type[] baseInterfaces);

TypeInfo GetProxyType(System.Type baseType, params System.Type[] baseInterfaces);

bool TryGetProxyType(System.Type baseType, System.Type[] baseInterfaces, out TypeInfo proxyType);

void StoreProxyType(TypeInfo result, System.Type baseType, params System.Type[] baseInterfaces);
}
}
}
20 changes: 7 additions & 13 deletions src/NHibernate/Proxy/DynamicProxy/ProxyCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,28 @@

#endregion

using System.Collections.Concurrent;
using System;
using System.Reflection;

namespace NHibernate.Proxy.DynamicProxy
{
//Since v5.1
[Obsolete("This class is not used anymore and will be removed in a next major version")]
public class ProxyCache : IProxyCache
{
private static readonly ConcurrentDictionary<ProxyCacheEntry, TypeInfo> cache = new ConcurrentDictionary<ProxyCacheEntry, TypeInfo>();

#region IProxyCache Members

public bool Contains(System.Type baseType, params System.Type[] baseInterfaces)
{
if (baseType == null)
{
return false;
}

var entry = new ProxyCacheEntry(baseType, baseInterfaces);
return cache.ContainsKey(entry);
return ProxyFactory._cache.ContainsKey(entry);
}

public TypeInfo GetProxyType(System.Type baseType, params System.Type[] baseInterfaces)
{
var entry = new ProxyCacheEntry(baseType, baseInterfaces);
return cache[entry];
return ProxyFactory._cache[entry];
}

public bool TryGetProxyType(System.Type baseType, System.Type[] baseInterfaces, out TypeInfo proxyType)
Expand All @@ -42,15 +38,13 @@ public bool TryGetProxyType(System.Type baseType, System.Type[] baseInterfaces,
return false;

var entry = new ProxyCacheEntry(baseType, baseInterfaces);
return cache.TryGetValue(entry, out proxyType);
return ProxyFactory._cache.TryGetValue(entry, out proxyType);
}

public void StoreProxyType(TypeInfo result, System.Type baseType, params System.Type[] baseInterfaces)
{
var entry = new ProxyCacheEntry(baseType, baseInterfaces);
cache[entry] = result;
ProxyFactory._cache[entry] = result;
}

#endregion
}
}
35 changes: 18 additions & 17 deletions src/NHibernate/Proxy/DynamicProxy/ProxyCacheEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,32 @@

using System;
using System.Collections.Generic;
using System.Linq;

namespace NHibernate.Proxy.DynamicProxy
{
public class ProxyCacheEntry : IEquatable<ProxyCacheEntry>
{
private readonly int _hashCode;
private readonly HashSet<System.Type> _uniqueInterfaces;

public ProxyCacheEntry(System.Type baseType, System.Type[] interfaces)
{
if (baseType == null)
throw new ArgumentNullException(nameof(baseType));
BaseType = baseType;
_uniqueInterfaces = new HashSet<System.Type>(interfaces ?? new System.Type[0]);
BaseType = baseType ?? throw new ArgumentNullException(nameof(baseType));

if (_uniqueInterfaces.Count == 0)
var uniqueInterfaces = new HashSet<System.Type>();
if (interfaces != null && interfaces.Length > 0)
{
_hashCode = baseType.GetHashCode();
return;
uniqueInterfaces.UnionWith(interfaces.Where(i => i != null));
}

var allTypes = new List<System.Type>(_uniqueInterfaces) { baseType };
_hashCode = 59;
foreach (System.Type type in allTypes)
_uniqueInterfaces = uniqueInterfaces;

_hashCode = 59 ^ baseType.GetHashCode();

if (_uniqueInterfaces.Count == 0)
return;

foreach (var type in _uniqueInterfaces)
{
// This simple implementation is nonsensitive to list order. If changing it for a sensitive one,
// take care of ordering the list.
Expand All @@ -39,14 +42,12 @@ public ProxyCacheEntry(System.Type baseType, System.Type[] interfaces)
}

public System.Type BaseType { get; }
public IReadOnlyCollection<System.Type> Interfaces { get { return _uniqueInterfaces; } }

private HashSet<System.Type> _uniqueInterfaces;

public IReadOnlyCollection<System.Type> Interfaces => _uniqueInterfaces;

public override bool Equals(object obj)
{
var that = obj as ProxyCacheEntry;
return Equals(that);
return Equals(obj as ProxyCacheEntry);
}

public bool Equals(ProxyCacheEntry other)
Expand All @@ -64,4 +65,4 @@ public bool Equals(ProxyCacheEntry other)

public override int GetHashCode() => _hashCode;
}
}
}
Loading