Repository navigation
runtime checkable protocols potential raising error with custom getattribute #105134
Description
Activity
Thanks! Looks like the root cause here is probably a bug in
inspect.getattr_staticrather thantyping. That does pose interesting questions for the typing_extensions backport, though.One thing I noticed from exploring DictWrapper is it strangely satisfies
d = _DictWrapper() assert isinstance(d, dict) # Passes assert issubclass(_DictWrapper, dict) # Fails
This looks to come from library wrapt being used to make DictWrapper behave like a dict. It relies mainly on ObjectProxy for this. My rough gut is there's likely a reproducing example only using wrapt/ObjectProxy without needing tensorflow. It feels like it partially proxies dict but not enough here.
Yes, so here's a repro that doesn't involve
typing, justinspect.getattr_static(using Python 3.11, sincetensorflowdoesn't install using CPythonmain):>>> import inspect >>> from tensorflow.python.trackable.data_structures import _DictWrapper >>> inspect.getattr_static(_DictWrapper(), 'bar') Traceback (most recent call last): File "<stdin>", line 1, in <module> File "C:\Users\alexw\AppData\Local\Programs\Python\Python311\Lib\inspect.py", line 1825, in getattr_static instance_result = _check_instance(obj, attr) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\alexw\AppData\Local\Programs\Python\Python311\Lib\inspect.py", line 1772, in _check_instance instance_dict = object.__getattribute__(obj, "__dict__") ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TypeError: this __dict__ descriptor does not support '_DictWrapper' objects
I think there's probably bugs in CPython and [either tensorflow or wrapt] here. The
_DictWrapperclass must be pretty weird ifobject.__getattribute__raisesTypeError. But also,inspect.getattr_staticshould be resilient to this kind of edge case: it should probably never raiseTypeError.Interesting! It looks like
inspect.getattr_staticis buggy on <=3.11, but not on 3.12+. Here's a repro on 3.11 that just involveswrapt(notensorflowcode):>>> import inspect, wrapt >>> class Foo: pass ... >>> class Bar(Foo, wrapt.ObjectProxy): pass ... >>> inspect.getattr_static(Bar({}), 'bar') Traceback (most recent call last): File "<stdin>", line 1, in <module> File "C:\Users\alexw\AppData\Local\Programs\Python\Python311\Lib\inspect.py", line 1825, in getattr_static instance_result = _check_instance(obj, attr) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\alexw\AppData\Local\Programs\Python\Python311\Lib\inspect.py", line 1772, in _check_instance instance_dict = object.__getattribute__(obj, "__dict__") ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TypeError: this __dict__ descriptor does not support 'Bar' objects
But on
main, it works as expected:>>> import inspect, wrapt >>> class Foo: pass ... >>> class Bar(Foo, wrapt.ObjectProxy): pass ... >>> inspect.getattr_static(Bar({}), 'bar') Traceback (most recent call last): File "<stdin>", line 1, in <module> File "C:\Users\alexw\coding\cpython\Lib\inspect.py", line 1867, in getattr_static raise AttributeError(attr) AttributeError: bar
Lots of changes have been made to
inspect.getattr_staticin Python 3.12 (in the name of improving performance -- see #103193), so it's entirely possible that one of these changes "accidentally" fixed this bug.It reproduces with the
wraptC extension, but not without it. Which might be why it reproduces on 3.11, but notmain...Here's a repro that doesn't involve
tensorfloworwrapt: https://github.com/AlexWaygood/getattr-static-repro.Unfortunately it involves a 3,000-line C extension (I've just vendored
wrapt's C extension to create the reproducible example)! I doubt I'll be the best person to help narrow this down further, since I'm no good with C. Maybe @carljm or @JelleZijlstra would be able to help narrow this down further?This issue looks pretty related:
The
TypeErrorhere is becauseinspect.getattr_staticis doingobject.__getattribute__(x, '__dict__'), and wherexis an instance ofwrapt.ObjectProxy, that can raiseTypeErrorif thewraptC extension is installed.I think it's only possible for
object.__getattribute__(x, '__dict__')to raiseTypeErrorifxhas been created via a custom C extension. And I think that means thatinspect.getattr_staticis doing nothing wrong here, and there is only a bug inwrapt. But, very interested to hear if @carljm has any thoughts on that!Haven't had a chance to investigate much, but here's the C stack trace (on main) for where the
this __dict__ descriptor does not support 'Bar' objectserror is raised:frame #3: 0x00000001000f3474 python`raise_dict_descr_error(obj=<unavailable>) at typeobject.c:2873:5 [opt] frame #4: 0x00000001000f3234 python`subtype_dict(obj=0x000000010317b9d0, context=<unavailable>) at typeobject.c:0 [opt] frame #5: 0x0000000100090e48 python`getset_get(descr=0x000000010319dd90, obj=0x000000010317b9d0, type=<unavailable>) at descrobject.c:203:16 [opt] frame #6: 0x00000001000d3008 python`_PyObject_GenericGetAttrWithDict(obj=0x000000010317b9d0, name=0x000000010050ba88, dict=0x0000000000000000, suppress=0) at object.c:0 [opt] frame #7: 0x00000001000d2e68 python`PyObject_GenericGetAttr(obj=<unavailable>, name=<unavailable>) at object.c:1515:12 [opt] frame #8: 0x00000001000e9c28 python`wrap_binaryfunc(self=0x000000010317b9d0, args=0x000000010317a0d0, wrapped=0x00000001000d2e54) at typeobject.c:7724:12 [opt] frame #9: 0x0000000100092cc0 python`wrapperdescr_raw_call(descr=0x0000000100e00290, self=0x000000010317b9d0, args=0x000000010317a0d0, kwds=0x0000000000000000) at descrobject.c:537:12 [opt] frame #10: 0x000000010009100c python`wrapperdescr_call(descr=0x0000000100e00290, args=0x000000010317a0d0, kwds=0x0000000000000000) at descrobject.c:574:14 [opt] frame #11: 0x00000001000861b8 python`_PyObject_MakeTpCall(tstate=0x00000001005751b8, callable=0x0000000100e00290, args=0x0000000100a7c1c8, nargs=<unavailable>, keywords=0x0000000000000000) at call.c:240:18 [opt] frame #12: 0x0000000100085f28 python`_PyObject_VectorcallTstate(tstate=0x00000001005751b8, callable=0x0000000100e00290, args=0x0000000100a7c1c8, nargsf=9223372036854775810, kwnames=0x0000000000000000) at pycore_call.h:90:16 [opt] frame #13: 0x0000000100086a04 python`PyObject_Vectorcall(callable=<unavailable>, args=<unavailable>, nargsf=<unavailable>, kwnames=<unavailable>) at call.c:325:12 [opt]I have this on my list to take a look at, but it may be some time next week before I can get to it.
Reacted by Alex WaygoodI looked into this a bit. Here's what's happening:
Foois a normal Python type, which has a__dict__descriptor (it only inheritsobject, whosetp_dictoffsetis0, so it getsPy_TPFLAGS_MANAGED_DICTand atp_dictoffsetof-1, so itstp_getsetincludes__dict__, which means that its own type dict gets populated with the default descriptor for__dict__bytype_add_getset).Bar, on the other hand, inherits the builtin typeWraptObjectProxy_Type(it also inheritsFoo, butWraptObjectProxy_Typeis the nearest base defining a C instance layout -- seebest_base-- so it is used to define the layout forBar), whosetp_dictoffsetis non-zero, thereforeBardoesn't getPy_TPFLAGS_MANAGED_DICTand its owntp_dictoffsetis left at0, therefore (seetype_new_set_slots) it does not get a__dict__descriptor in its type dict.- The MRO of
Baris(Bar, Foo, WraptObjectProxy_Type, object). - So (in
inspect.getattr_static) when we look up__dict__in the type dictionary ofBar, we don't find it inBar.__dict__, we go up the MRO toFoo, and we do find the default__dict__descriptor inFoo.__dict__. - When we actually try to run this descriptor's getter,
get_builtin_base_with_dictfindsWraptObjectProxy_Typeas the nearest builtin base type with a dict, and so we try to use its__dict__descriptor. But it doesn't have one! (No entry for__dict__inWraptObjectProxy_getset.) Thus we get the error.
I'm not 100% sure about the logic behind
get_builtin_base_with_dict; it seems a bit ad-hoc. But it's also been there a long time and would be very disruptive to change, so I'm not going to think too hard about changing it. If we accept its implementation as given, that implies that this is a bug inWraptObjectProxy: C extension types that definetp_dictoffsetreally need to also define a__dict__descriptor in order to avoid this problem.Given all that, the question remains about what
inspect.getattr_staticshould do when it encounters a type that is broken in this way. On one hand, the whole point ofinspect.getattr_staticis to check for existence of an attribute "safely." But on the other hand, I think this particular TypeError pretty much always indicates a bug in the C extension type, and I don't like papering over bugs and allowing them to persist longer undetected. In this case, if we catch theTypeErrorthe result would beinspect.getattr_staticwould always fail to find instance attributes on instances of such broken types; this seems even more confusing than raising theTypeError. And I think the proper definition of "safely" forinspect.getattr_staticis not "can never raise" but rather "doesn't execute arbitrary Python code," and no arbitrary Python code is being executed here. Raising in case of a badly-defined type seems to me still the best option forinspect.getattr_static.So my recommendation would be to close this issue as not a bug in Python.
Reacted by Alex Waygood and Jelle ZijlstraThanks so much for the in-depth investigation @carljm! This confirms my suspicions, and I agree with your conclusion.
Bug report
I'm unsure if this is more tensorflow bug or python bug but it does trace to this pr and error message traces to cpython source. I've also commented on issue in tensorflow side here.
With typing extensions an error message is produced of,
Your environment
I've tested on cpython 3.9.16 and 3.10.10 where using typing works. I'd suspect 3.11 will work as well. Using typing_extensions 4.6 triggers error with inspect.getattr_static usage. Reported here instead of typing extensions as adding getattr static there was backport from here. I'd expect this to reproduce on 3.12 although installing tf on 3.12 is likely tricky and easier to test with typing extensions. The _DictWrapper class is here and is like a subclass of
__dict__with a custom getattribute to do some extra tracking of elements.I used latest version of tensorflow (2.12). The underlying error message comes from this line. As this can be triggered in attribute access unsure whether it should be TypeError vs AttributeError. If it was attribute error getattr static would catch it and be fine. If it needs to be type error then it's unclear to me whether tensorflow class is breaking some assumption or if getattr static should also catch type errors here. In particular it's interesting that
object.__getattribute__can raise TypeError and inspect.getattr_static can fail.I'll try to look tomorrow if I can make a more self contained example that doesn't require tensorflow.