Use a native __getattr__ hook for AttributeError suggestions (GIL-safe re-land of the miss-only hook)

src/runtime/AttributeErrorHint.cs

@@ -0,0 +1,163 @@
+using System;
+using System.Reflection;
+using System.Runtime.InteropServices;
+
+using Python.Runtime.Native;
+
+namespace Python.Runtime
+{
+    /// <summary>
+    /// Installs a miss-only <c>__getattr__</c> hook on reflected .NET types so that an
+    /// <c>AttributeError</c> raised for a missing attribute is enriched with suggestions
+    /// of similarly-named members — without adding any cost to the (common) successful
+    /// attribute-access path.
+    /// </summary>
+    /// <remarks>
+    /// CPython only invokes <c>__getattr__</c> after the normal attribute lookup fails,
+    /// via the native <c>slot_tp_getattr_hook</c>: on a hit it calls the generic getattr
+    /// directly (no managed transition); only on a miss does it call our <c>__getattr__</c>.
+    /// pythonnet's metatype does not run CPython's slot-fixup machinery when an attribute
+    /// is set on a type, so simply adding <c>__getattr__</c> to the type dict would not
+    /// rewire the slot — we therefore wire <c>tp_getattro</c> to the hook manually.
+    ///
+    /// The <c>__getattr__</c> itself is a native method descriptor (PyDescr_NewMethod)
+    /// around a managed thunk, NOT a .NET delegate exposed to Python: delegate calls go
+    /// through <see cref="MethodBinder"/>, which releases the GIL around the invocation
+    /// (allow_threads), so the callback would run CPython C-API calls off-GIL and crash
+    /// whenever the <see cref="Finalizer"/> fires mid-callback. The native thunk is
+    /// called directly by the interpreter with the GIL held.
+    /// </remarks>
+    internal static class AttributeErrorHint
+    {
+        // Unmanaged PyMethodDef backing the shared __getattr__ method descriptors.
+        // Descriptors keep a raw pointer to it (d_method) and can outlive engine
+        // shutdown bookkeeping, so it is allocated once and kept for the process
+        // lifetime (as are the thunks in Interop.allocatedThunks).
+        private static IntPtr _methodDef;
+        // Keeps the thunk delegate for GetAttrHook alive.
+        private static ThunkInfo? _thunk;
+        // Address of CPython's slot_tp_getattr_hook (extracted from a probe type).
+        private static IntPtr _hookSlot;
+        // Address of PyObject_GenericGetAttr, used to detect types we may safely redirect.
+        private static IntPtr _genericGetAttr;
+
+        private static bool IsReady => _methodDef != IntPtr.Zero && _hookSlot != IntPtr.Zero;
+
+        internal static void Initialize()
+        {
+            try
+            {
+                _genericGetAttr = Util.ReadIntPtr(Runtime.PyBaseObjectType, TypeOffset.tp_getattro);
+
+                if (_methodDef == IntPtr.Zero)
+                {
+                    _thunk = Interop.GetThunk(typeof(AttributeErrorHint).GetMethod(
+                        nameof(GetAttrHook), BindingFlags.Static | BindingFlags.Public)!);
+                    IntPtr methodDef = Marshal.AllocHGlobal(4 * IntPtr.Size);
+                    TypeManager.WriteMethodDef(methodDef, "__getattr__", _thunk.Address);
+                    _methodDef = methodDef;
+                }
+
+                // Define a probe class whose tp_getattro is slot_tp_getattr_hook so we
+                // can read that function pointer.
+                using var globals = new PyDict();
+                Runtime.PyDict_SetItemString(globals.Reference, "__builtins__", Runtime.PyEval_GetBuiltins());
+                PythonEngine.Exec(
+                    "class __clr_getattr_probe__:\n" +
+                    "    def __getattr__(self, name):\n" +
+                    "        raise AttributeError(name)\n",
+                    globals);
+
+                using var probe = globals["__clr_getattr_probe__"];
+                _hookSlot = Util.ReadIntPtr(probe.Reference, TypeOffset.tp_getattro);
+            }
+            catch (Exception e)
+            {
+                // Degrade gracefully: without the hook, AttributeError messages are simply
+                // not enriched. Never let this break interpreter initialization.
+                DebugUtil.Print($"AttributeErrorHint.Initialize failed: {e}");
+                Shutdown();
+            }
+        }
+
+        /// <summary>
+        /// Wires the miss-only hook onto <paramref name="type"/> if it still uses the
+        /// native generic getattr. Types with a custom <c>tp_getattro</c> (dynamic
+        /// objects, modules, interfaces, ...) handle misses themselves and are left
+        /// untouched; derived types that inherit an already-hooked base are likewise
+        /// skipped, since they inherit the behavior through the MRO.
+        /// </summary>
+        internal static void Install(BorrowedReference type)
+        {
+            if (!IsReady)
+            {
+                return;
+            }
+
+            if (Util.ReadIntPtr(type, TypeOffset.tp_getattro) != _genericGetAttr)
+            {
+                return;
+            }
+
+            using var descr = Runtime.PyDescr_NewMethod(type, _methodDef);
+            if (descr.IsNull())
+            {
+                Exceptions.Clear();
+                return;
+            }
+
+            BorrowedReference dict = Util.ReadRef(type, TypeOffset.tp_dict);
+            if (Runtime.PyDict_SetItemString(dict, "__getattr__", descr.Borrow()) != 0)
+            {
+                Exceptions.Clear();
+                return;
+            }
+
+            Util.WriteIntPtr(type, TypeOffset.tp_getattro, _hookSlot);
+            Runtime.PyType_Modified(type);
+        }
+
+        /// <summary>
+        /// The <c>__getattr__(self, name)</c> implementation (METH_VARARGS). CPython's
+        /// <c>slot_tp_getattr_hook</c> only calls it after the normal lookup has failed
+        /// and the original AttributeError has been cleared, so the full message is
+        /// rebuilt here. Runs as a direct native method call with the GIL held.
+        /// </summary>
+        public static NewReference GetAttrHook(BorrowedReference ob, BorrowedReference args)
+        {
+            string? name = null;
+            string message;
+            try
+            {
+                if (Runtime.PyTuple_Size(args) == 1)
+                {
+                    BorrowedReference key = Runtime.PyTuple_GetItem(args, 0);
+                    if (Runtime.PyString_Check(key))
+                    {
+                        name = Runtime.GetManagedString(key);
+                    }
+                }
+
+                using var self = new PyObject(ob);
+                message = ClassBase.BuildMissingAttributeMessage(self, name ?? "?");
+            }
+            catch
+            {
+                // Never let message building turn into a different exception.
+                message = $"object has no attribute '{name ?? "?"}'";
+            }
+
+            Exceptions.SetError(Exceptions.AttributeError, message);
+            return default;
+        }
+
+        internal static void Shutdown()
+        {
+            // _methodDef and _thunk are deliberately kept: method descriptors created
+            // from them may still be reachable during interpreter teardown, and both
+            // are reused by the next Initialize.
+            _hookSlot = IntPtr.Zero;
+            _genericGetAttr = IntPtr.Zero;
+        }
+    }
+}

src/runtime/PythonEngine.cs

@@ -263,6 +263,10 @@ public static void Initialize(IEnumerable<string> args, bool setSysArgv = true,
             }
 
             ImportHook.UpdateCLRModuleDict();
+
+            // Set up the miss-only __getattr__ hook used to enrich AttributeError
+            // messages on reflected .NET types with member-name suggestions.
+            AttributeErrorHint.Initialize();
         }
 
         static BorrowedReference DefineModule(string name)
@@ -369,6 +373,9 @@ public static void Shutdown()
             AppDomain.CurrentDomain.ProcessExit -= OnProcessExit;
 
             ExecuteShutdownHandlers();
+
+            AttributeErrorHint.Shutdown();
+
             // Remember to shut down the runtime.
             Runtime.Shutdown();
 

src/runtime/Runtime.Delegates.cs

@@ -230,6 +230,7 @@ static Delegates()
             PyObject_GenericGetAttr = (delegate* unmanaged[Cdecl]<BorrowedReference, BorrowedReference, NewReference>)GetFunctionByName(nameof(PyObject_GenericGetAttr), GetUnmanagedDll(_PythonDll));
             PyObject_GenericGetDict = (delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, NewReference>)GetFunctionByName(nameof(PyObject_GenericGetDict), GetUnmanagedDll(PythonDLL));
             PyObject_GenericSetAttr = (delegate* unmanaged[Cdecl]<BorrowedReference, BorrowedReference, BorrowedReference, int>)GetFunctionByName(nameof(PyObject_GenericSetAttr), GetUnmanagedDll(_PythonDll));
+            PyDescr_NewMethod = (delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, NewReference>)GetFunctionByName(nameof(PyDescr_NewMethod), GetUnmanagedDll(_PythonDll));
             PyObject_GC_Del = (delegate* unmanaged[Cdecl]<StolenReference, void>)GetFunctionByName(nameof(PyObject_GC_Del), GetUnmanagedDll(_PythonDll));
             try
             {
@@ -504,6 +505,7 @@ static Delegates()
         internal static delegate* unmanaged[Cdecl]<BorrowedReference, BorrowedReference, BorrowedReference> _PyType_Lookup { get; }
         internal static delegate* unmanaged[Cdecl]<BorrowedReference, BorrowedReference, NewReference> PyObject_GenericGetAttr { get; }
         internal static delegate* unmanaged[Cdecl]<BorrowedReference, BorrowedReference, BorrowedReference, int> PyObject_GenericSetAttr { get; }
+        internal static delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, NewReference> PyDescr_NewMethod { get; }
         internal static delegate* unmanaged[Cdecl]<StolenReference, void> PyObject_GC_Del { get; }
         internal static delegate* unmanaged[Cdecl]<BorrowedReference, int> PyObject_GC_IsTracked { get; }
         internal static delegate* unmanaged[Cdecl]<BorrowedReference, void> PyObject_GC_Track { get; }

src/runtime/Runtime.cs

@@ -1738,6 +1738,13 @@ internal static bool PyType_IsSameAsOrSubtype(BorrowedReference type, BorrowedRe
 
         internal static int PyObject_GenericSetAttr(BorrowedReference obj, BorrowedReference name, BorrowedReference value) => Delegates.PyObject_GenericSetAttr(obj, name, value);
 
+
+        /// <summary>
+        /// Creates a method descriptor for <paramref name="type"/> from an unmanaged
+        /// <c>PyMethodDef*</c>, which must remain valid for the descriptor's lifetime.
+        /// </summary>
+        internal static NewReference PyDescr_NewMethod(BorrowedReference type, IntPtr methodDef) => Delegates.PyDescr_NewMethod(type, methodDef);
+
         internal static NewReference PyObject_GenericGetDict(BorrowedReference o) => PyObject_GenericGetDict(o, IntPtr.Zero);
         internal static NewReference PyObject_GenericGetDict(BorrowedReference o, IntPtr context) => Delegates.PyObject_GenericGetDict(o, context);
 

src/runtime/TypeManager.cs

@@ -303,6 +303,10 @@ internal static void InitializeClass(PyType type, ClassBase impl, Type clrType)
 
             Runtime.PyType_Modified(type.Reference);
 
+            // Enrich AttributeError messages for missing attributes with member-name
+            // suggestions, via a miss-only __getattr__ hook (no hot-path cost).
+            AttributeErrorHint.Install(type.Reference);
+
             //DebugUtil.DumpType(type);
         }
 

src/runtime/Types/ClassBase.cs

@@ -628,20 +628,13 @@ internal static void AppendAttributeErrorSuggestions(BorrowedReference ob, Borro
             }
 
             var name = Runtime.GetManagedString(key);
-            // Skip empty and dunder names: the latter are probed internally by CPython
-            // (e.g. __iter__, __len__) and are never user-facing typos worth helping with.
-            if (string.IsNullOrEmpty(name) || name.StartsWith("__", StringComparison.Ordinal))
+            if (string.IsNullOrEmpty(name))
             {
                 return;
             }
 
-            if (GetManagedObject(ob) is not CLRObject clrObj || clrObj.inst is null)
-            {
-                return;
-            }
-
-            var suggestions = GetSimilarMemberNames(clrObj.inst.GetType(), name);
-            if (suggestions.Count == 0)
+            var hint = GetSuggestionHint(ob, name);
+            if (hint.Length == 0)
             {
                 return;
             }
@@ -651,7 +644,6 @@ internal static void AppendAttributeErrorSuggestions(BorrowedReference ob, Borro
             try
             {
                 var baseMessage = GetErrorMessage(errValue.BorrowNullable(), name);
-                var hint = " Did you mean: " + string.Join(", ", suggestions.Select(s => $"'{s}'")) + "?";
                 Exceptions.SetError(Exceptions.AttributeError, baseMessage + hint);
             }
             finally
@@ -662,6 +654,65 @@ internal static void AppendAttributeErrorSuggestions(BorrowedReference ob, Borro
             }
         }
 
+        /// <summary>
+        /// Builds the full message for an <c>AttributeError</c> raised for a missing
+        /// attribute on a .NET object, including any "Did you mean ...?" hint. Used by
+        /// the miss-only <c>__getattr__</c> hook installed on reflected types (see
+        /// <see cref="AttributeErrorHint"/>), where the original error has already been
+        /// cleared, so the base message is reconstructed here.
+        /// </summary>
+        internal static string BuildMissingAttributeMessage(PyObject self, string name)
+        {
+            var typeName = "object";
+            try
+            {
+                using var pyType = self.GetPythonType();
+                typeName = pyType.Name;
+            }
+            catch
+            {
+                // fall back to the generic type name
+            }
+
+            var message = $"'{typeName}' object has no attribute '{name}'";
+            try
+            {
+                return message + GetSuggestionHint(self.Reference, name);
+            }
+            catch
+            {
+                // never let suggestion building turn into a different exception
+                return message;
+            }
+        }
+
+        /// <summary>
+        /// Returns " Did you mean: 'x', 'y'?" listing similarly-named members of the
+        /// managed object, or an empty string when there is nothing to suggest. Dunder
+        /// names are skipped: they are probed internally by CPython (e.g. __iter__,
+        /// __len__) and are never user-facing typos worth helping with.
+        /// </summary>
+        private static string GetSuggestionHint(BorrowedReference ob, string name)
+        {
+            if (string.IsNullOrEmpty(name) || name.StartsWith("__", StringComparison.Ordinal))
+            {
+                return string.Empty;
+            }
+
+            if (GetManagedObject(ob) is not CLRObject clrObj || clrObj.inst is null)
+            {
+                return string.Empty;
+            }
+
+            var suggestions = GetSimilarMemberNames(clrObj.inst.GetType(), name);
+            if (suggestions.Count == 0)
+            {
+                return string.Empty;
+            }
+
+            return " Did you mean: " + string.Join(", ", suggestions.Select(s => $"'{s}'")) + "?";
+        }
+
         private static string GetErrorMessage(BorrowedReference value, string fallbackName)
         {
             if (value != null)

src/runtime/Types/ClassObject.cs

@@ -167,21 +167,6 @@ public override void InitializeSlots(BorrowedReference pyType, SlotsHolder slots
         protected virtual NewReference NewObjectToPython(object obj, BorrowedReference tp)
             => CLRObject.GetReference(obj, tp);
 
-        /// <summary>
-        /// Type __getattro__ implementation. Delegates to the generic CLR attribute
-        /// lookup, but enriches the AttributeError raised for a missing attribute with
-        /// suggestions of similarly-named members of the managed type.
-        /// </summary>
-        public static NewReference tp_getattro(BorrowedReference ob, BorrowedReference key)
-        {
-            var result = Runtime.PyObject_GenericGetAttr(ob, key);
-            if (result.IsNull())
-            {
-                AppendAttributeErrorSuggestions(ob, key);
-            }
-            return result;
-        }
-
         private static NewReference NewEnum(Type type, BorrowedReference args, BorrowedReference tp)
         {
             nint argCount = Runtime.PyTuple_Size(args);

tests/test_class.py

@@ -104,6 +104,23 @@ def test_missing_attribute_hasattr_still_false():
     assert hasattr(s, "Length")
 
 
+def test_missing_attribute_hook_is_native():
+    """The __getattr__ hook must be a native method descriptor.
+
+    If it were a Python function calling into .NET through a delegate, the call
+    would go through MethodBinder, which releases the GIL around the invocation:
+    the hint-building callback would then run CPython C-API calls off-GIL and
+    crash whenever the pythonnet Finalizer fires mid-callback (the Lean CI crash
+    on 2.0.55).
+    """
+    hook = next(
+        c.__dict__["__getattr__"]
+        for c in type(System.String("x")).__mro__
+        if "__getattr__" in c.__dict__
+    )
+    assert type(hook).__name__ == "method_descriptor"
+
+
 def test_basic_subclass():
     """Test basic subclass of a managed class."""
     from System.Collections import Hashtable