runtime checkable protocols potential raising error with custom getattribute

[hmc-cs-mdrissi]

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.

# typing 3.9/3.10 produces no error, typing_extensions will trigger it.
from typing_extensions import Protocol, runtime_checkable

from tensorflow.python.trackable.data_structures import _DictWrapper


@runtime_checkable
class Foo(Protocol):
    def bar(self) -> None:
        ...


isinstance(_DictWrapper(), Foo)

With typing extensions an error message is produced of,

Traceback (most recent call last):
  File "/Users/pa-loaner/Snapchat/Dev/training-platform/scratch/runtime_protocol_custom_dict.py", line 12, in <module>
    isinstance(_DictWrapper(), Foo)
  File "/Users/pa-loaner/Snapchat/Dev/.venvs/tf212/lib/python3.9/site-packages/typing_extensions.py", line 604, in __instancecheck__
    val = inspect.getattr_static(instance, attr)
  File "/Users/pa-loaner/.pyenv/versions/3.9.16/lib/python3.9/inspect.py", line 1711, in getattr_static
    instance_result = _check_instance(obj, attr)
  File "/Users/pa-loaner/.pyenv/versions/3.9.16/lib/python3.9/inspect.py", line 1654, in _check_instance
    instance_dict = object.__getattribute__(obj, "__dict__")
TypeError: this __dict__ descriptor does not support '_DictWrapper' objects

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.

[AlexWaygood]

Thanks! Looks like the root cause here is probably a bug in inspect.getattr_static rather than typing. That does pose interesting questions for the typing_extensions backport, though.

[hmc-cs-mdrissi]

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.

[AlexWaygood]

Yes, so here's a repro that doesn't involve typing, just inspect.getattr_static (using Python 3.11, since tensorflow doesn't install using CPython main):

>>> 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 _DictWrapper class must be pretty weird if object.__getattribute__ raises TypeError. But also, inspect.getattr_static should be resilient to this kind of edge case: it should probably never raise TypeError.

[AlexWaygood]

Interesting! It looks like inspect.getattr_static is buggy on <=3.11, but not on 3.12+. Here's a repro on 3.11 that just involves wrapt (no tensorflow code):

>>> 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_static in 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.

[AlexWaygood]

It reproduces with the wrapt C extension, but not without it. Which might be why it reproduces on 3.11, but not main...

[AlexWaygood]

Here's a repro that doesn't involve tensorflow or wrapt: 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?

[AlexWaygood]

This issue looks pretty related:

• this __dict__ descriptor does not support '_DictWrapper' objects \ since version 1.15.0 GrahamDumpleton/wrapt#231

[AlexWaygood]

The TypeError here is because inspect.getattr_static is doing object.__getattribute__(x, '__dict__'), and where x is an instance of wrapt.ObjectProxy, that can raise TypeError if the wrapt C extension is installed.

I think it's only possible for object.__getattribute__(x, '__dict__') to raise TypeError if x has been created via a custom C extension. And I think that means that inspect.getattr_static is doing nothing wrong here, and there is only a bug in wrapt. But, very interested to hear if @carljm has any thoughts on that!