Make object destructors not proxiable (#3233)

Fixes #3205
nhibernate · Apr 3, 2023 · a98192d · a98192d
1 parent 160e35d
commit a98192d
Show file tree

Hide file tree

Showing 3 changed files with 131 additions and 9 deletions.
diff --git a/src/NHibernate.Test/NHSpecificTest/ProxyValidator/ShouldBeProxiableTests.cs b/src/NHibernate.Test/NHSpecificTest/ProxyValidator/ShouldBeProxiableTests.cs
@@ -8,12 +8,27 @@ namespace NHibernate.Test.NHSpecificTest.ProxyValidator
  [TestFixture]
  public class ShouldBeProxiableTests
  {
- private class MyClass: IDisposable
+ private class MyClass : IDisposable
  {
  public void Dispose()
  {
  }
+
+ ~MyClass()
+ {
+ }
+
+ // ReSharper disable once InconsistentNaming
+ // This is intentionally lower case
+ public virtual void finalize()
+ {
+ }
+
+ public virtual void Finalize(int a)
+ {
+ }
  }
+
  private class ProtectedNoVirtualProperty
  {
  protected int Aprop { get; set; }
@@ -44,6 +59,104 @@ public void DisposeNotBeProxiable()
  Assert.That(method.ShouldBeProxiable(), Is.False);
  }
 
+ [Test]
+ public void ObjectDestructorShouldNotBeProxiable()
+ {
+ var method = typeof(object).GetMethod(
+ "Finalize",
+ BindingFlags.NonPublic | BindingFlags.Instance);
+
+ Assert.That(method.ShouldBeProxiable(), Is.False);
+ }
+
+ [Test]
+ public void ObjectDestructorIsNotProxiable()
+ {
+ var method = typeof(object).GetMethod(
+ "Finalize",
+ BindingFlags.NonPublic | BindingFlags.Instance);
+
+ Assert.That(method.IsProxiable(), Is.False);
+ }
+
+ [Test]
+ public void MyClassDestructorShouldNotBeProxiable()
+ {
+ var method = typeof(MyClass).GetMethod(
+ "Finalize",
+ BindingFlags.NonPublic | BindingFlags.Instance,
+ null,
+ System.Type.EmptyTypes,
+ null);
+
+ Assert.That(method.ShouldBeProxiable(), Is.False);
+ }
+
+ [Test]
+ public void MyClassDestructorIsNotProxiable()
+ {
+ var method = typeof(MyClass).GetMethod(
+ "Finalize",
+ BindingFlags.NonPublic | BindingFlags.Instance,
+ null,
+ System.Type.EmptyTypes,
+ null);
+
+ Assert.That(method.IsProxiable(), Is.False);
+ }
+
+ [Test]
+ public void MyClassLowerCaseFinalizeShouldBeProxiable()
+ {
+ var method = typeof(MyClass).GetMethod(
+ "finalize",
+ BindingFlags.Public | BindingFlags.Instance,
+ null,
+ System.Type.EmptyTypes,
+ null);
+
+ Assert.That(method.ShouldBeProxiable(), Is.True);
+ }
+
+ [Test]
+ public void MyClassLowerCaseFinalizeIsProxiable()
+ {
+ var method = typeof(MyClass).GetMethod(
+ "finalize",
+ BindingFlags.Public | BindingFlags.Instance,
+ null,
+ System.Type.EmptyTypes,
+ null);
+
+ Assert.That(method.IsProxiable(), Is.True);
+ }
+
+ [Test]
+ public void MyClassFinalizeWithParametersShouldBeProxiable()
+ {
+ var method = typeof(MyClass).GetMethod(
+ "Finalize",
+ BindingFlags.Public | BindingFlags.Instance,
+ null,
+ new[] { typeof(int) },
+ null);
+
+ Assert.That(method.ShouldBeProxiable(), Is.True);
+ }
+
+ [Test]
+ public void MyClassFinalizeWithParametersIsProxiable()
+ {
+ var method = typeof(MyClass).GetMethod(
+ "Finalize",
+ BindingFlags.Public | BindingFlags.Instance,
+ null,
+ new[] { typeof(int) },
+ null);
+
+ Assert.That(method.IsProxiable(), Is.True);
+ }
+
  [Test]
  public void WhenProtectedNoVirtualPropertyThenShouldntBeProxiable()
  {

diff --git a/src/NHibernate/Proxy/DefaultDynamicProxyMethodCheckerExtensions.cs b/src/NHibernate/Proxy/DefaultDynamicProxyMethodCheckerExtensions.cs
@@ -11,7 +11,7 @@ public static bool IsProxiable(this MethodInfo method)
  {
  return !method.IsFinal
  && (method.DeclaringType != typeof(MarshalByRefObject))
- && (method.DeclaringType != typeof(object) || !"finalize".Equals(method.Name, StringComparison.OrdinalIgnoreCase))
+ && !IsFinalizeMethod(method)
  &&
  (
  ((method.IsPublic || method.IsFamily) && (method.IsVirtual || method.IsAbstract)) // public or protected (virtual)
@@ -24,7 +24,7 @@ public static bool ShouldBeProxiable(this MethodInfo method)
  {
  // to use only for real methods (no getter/setter)
  return (method.DeclaringType != typeof (MarshalByRefObject)) &&
- (method.DeclaringType != typeof (object) || !"finalize".Equals(method.Name, StringComparison.OrdinalIgnoreCase)) &&
+ !IsFinalizeMethod(method) &&
  (!(method.DeclaringType == typeof (object) && "GetType".Equals(method.Name))) &&
  (!(method.DeclaringType == typeof (object) && "obj_address".Equals(method.Name))) && // Mono-specific method
  !IsDisposeMethod(method) &&
@@ -33,12 +33,10 @@ public static bool ShouldBeProxiable(this MethodInfo method)
 
  public static bool ShouldBeProxiable(this PropertyInfo propertyInfo)
  {
- if(propertyInfo != null)
- {
- var accessors = propertyInfo.GetAccessors(true);
- return accessors.Where(x => x.IsPublic || x.IsAssembly || x.IsFamilyOrAssembly).Any();
- }
- return true;
+ if (propertyInfo == null) return true;
+
+ var accessors = propertyInfo.GetAccessors(true);
+ return accessors.Any(x => x.IsPublic || x.IsAssembly || x.IsFamilyOrAssembly);
  }
 
  private static bool IsDisposeMethod(MethodInfo method)
@@ -47,5 +45,10 @@ private static bool IsDisposeMethod(MethodInfo method)
  return method.Name.Equals("Dispose") && method.MemberType == MemberTypes.Method && method.GetParameters().Length == 0;
  // return method.Name.Equals("Dispose") && method.IsMethodOf(typeof(IDisposable));
  }
+
+ private static bool IsFinalizeMethod(MethodInfo method)
+ {
+ return method.GetBaseDefinition() == ReflectionCache.ObjectMethods.Finalize;
+ }
  }
 }
diff --git a/src/NHibernate/Util/ReflectionCache.cs b/src/NHibernate/Util/ReflectionCache.cs
@@ -232,5 +232,11 @@ internal static class TypeMethods
  internal static readonly MethodInfo GetTypeFromHandle =
  ReflectHelper.FastGetMethod(System.Type.GetTypeFromHandle, default(RuntimeTypeHandle));
  }
+
+ internal static class ObjectMethods
+ {
+ internal static readonly MethodInfo Finalize =
+ typeof(object).GetMethod("Finalize", BindingFlags.NonPublic | BindingFlags.Instance);
+ }
  }
 }