From a98192dcce357263145d83b6914d87584f97a4e5 Mon Sep 17 00:00:00 2001 From: Alex Zaytsev Date: Tue, 4 Apr 2023 03:13:01 +1000 Subject: [PATCH] Make object destructors not proxiable (#3233) Fixes #3205 --- .../ProxyValidator/ShouldBeProxiableTests.cs | 115 +++++++++++++++++- ...aultDynamicProxyMethodCheckerExtensions.cs | 19 +-- src/NHibernate/Util/ReflectionCache.cs | 6 + 3 files changed, 131 insertions(+), 9 deletions(-) diff --git a/src/NHibernate.Test/NHSpecificTest/ProxyValidator/ShouldBeProxiableTests.cs b/src/NHibernate.Test/NHSpecificTest/ProxyValidator/ShouldBeProxiableTests.cs index 1a6fe621ccd..204a0c0997f 100644 --- 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 index 052833230fe..b25c01c9fea 100644 --- 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 index 47fde15950d..45f97559780 100644 --- 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); + } } }