reversed() does not work with weakref.proxy of sequences

[abingham]

BPO	19359
Nosy	@rhettinger, @pitrou, @vstinner, @benjaminp, @PCManticore, @iritkatriel
Files	reversed_proxy_failure.py: Demonstrates issue reversed_proxy.patch issue19359.patch: Remove trailing whitespace. reversed_list_proxy_test.py: Show that reversed isn't detected on a proxy

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/benjaminp'
closed_at = None
created_at = <Date 2013-10-23.07:00:08.425>
labels = ['3.8', 'type-bug', 'library', '3.9', '3.10']
title = 'reversed() does not work with weakref.proxy of sequences'
updated_at = <Date 2021-03-14.17:52:30.538>
user = 'https://bugs.python.org/abingham'

bugs.python.org fields:

activity = <Date 2021-03-14.17:52:30.538>
actor = 'iritkatriel'
assignee = 'benjamin.peterson'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2013-10-23.07:00:08.425>
creator = 'abingham'
dependencies = []
files = ['32309', '32322', '34376', '34413']
hgrepos = []
issue_num = 19359
keywords = ['patch']
message_count = 8.0
messages = ['201002', '201069', '213526', '213528', '213529', '213530', '213562', '388678']
nosy_count = 7.0
nosy_names = ['rhettinger', 'pitrou', 'vstinner', 'benjamin.peterson', 'Claudiu.Popa', 'abingham', 'iritkatriel']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue19359'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[abingham]

When passed a weakref.proxy to a legitimate sequence, reversed() throws a TypeError complaining that its argument isn't a sequence. Perhaps this is the expected behavior, but it's surprising to at least me and the few others I've spoken with about it.

[PCManticore]

Attached an attemptive patch.

[rhettinger]

Thanks for looking at this.

Putting in a special case for weak references isn't the way to go. The problem is that _PyObject_LookupSpecial isn't working well with weakref proxies.

A solution for reversed() could be to replace PyObject_GetAttrString(seq, "__reversed__") with the slower call to _PyObject_LookupSpecial(seq, "__reversed__", &reversed_cache);.

The unfortunate downside is that this would slow down the common cases.

Another solution is to patch _PyObject_LookupSpecial to make it smarter with respect to weakref objects or possibly use a fallback to PyObject_GetAttrString when a method isn't found using the speedy lookup.

The advantage of fixing _PyObject_LookupSpecial is that it fixes all uses of _PyObject_LookupSpecial not just reversed(). Also, it would keep the current speed benefit for the common case.

I'm reassigning to Benjamin because it was his optimized that broke the ability to find the __reversed__ method on a proxy object.

Another issue that needs to be looked at is whether PySequence_Check() needs to be made aware of weakref proxies.

[pitrou]

I suppose Benjamin's commit is afd62eb1692e.

Claudiu, that doesn't look like the best approach to me. Instead of hardcoding a weakref.proxy check in reversed(), why not implement __reversed__ on weakref.proxy?

[pitrou]

It would also perhaps be practical to have some kind of "proxy mixin" that everyone can re-use to avoid having to reimplement __special__ methods by hand.

[pitrou]

Another possible idea is to introduce a "proxy protocol" (proxy /
tp_proxy) that would be used as a fallback by PyObject_LookupSpecial to
fetch the lookup target, i.e.:

def PyObject_LookupSpecial(obj, name):
    tp = type(obj)
    try:
        return getattr(tp, name)
    except AttributeError:
        return getattr(tp.tp_proxy(), name)

(not sure that makes sense, I haven't thought very hard about it)

[benjaminp]

afd62eb1692e wasn't a matter of speed, but a matter of correctness. Special methods should be looked up on the type on the instance as was done before.

[iritkatriel]

On 3.10 the reversed_proxy_failure.py script fails with

Traceback (most recent call last):
  File "/Users/iritkatriel/src/cpython/tmp.py", line 18, in <module>
    reversed(weakref.proxy(foo))  # TypeError
AttributeError: 'Foo' object has no attribute '__reversed__'

[iritkatriel]

This seems to have been fixed:

% cat xx.py 
from weakref import proxy

class List(list):
    pass

s = List([10, 20, 30, 40])
p = proxy(s)
print( list(reversed(p)))
% ./python.exe xx.py
[40, 30, 20, 10]

[terryjreedy]

The OP said, a bit ambiguously, that the reproduce raised a "TypeError complaining that its argument isn't a sequence." @Irit, in 2021, you noted that the reproducer failed on 3.10 with TypeError and AttributeError. Did you see this as equivalent to the original claim, verifying the bug, or not?

Two months ago you claimed that the bug was fixed because different code runs (in 3.13 or 3.12?). That same code, which fails with just TypeError in 3.8, runs in 3.9. The original reproducer still fails, in 3.13, as it did in 3.10 and 3.9. In 3.8, it failed with just a TypeError. So the conversion of TypeError to Attribute error happened in 3.9, but there is no evidence that anything has changed since then. Is it a bug that the original reproducer still fails, or is it invalid as a bug example?