Problem with wrapt.decorator and weakref.WeakMethod

[p7g]

Hi there,

I'm having an issue when trying to call a WeakMethod of a decorated bound function.

When trying to call the result of dereferencing the ref, it seems that the object to which the function is bound is what is called.

Here is a minimal reproduction that results in TypeError: 'TestClass' object is not callable on the last line:

import weakref
import wrapt


class TestClass:
	def testmethod(self):
		return id(self)


@wrapt.decorator
def pass_through(wrapped, instance, args, kwargs):
	return wrapped(*args, **kwargs)


instance = TestClass()

normal = instance.testmethod
normal_ref = weakref.WeakMethod(normal)

print('normal', normal())
print('normal ref', normal_ref()())

wrapped = pass_through(normal)
wrapped_ref = weakref.WeakMethod(wrapped)

print('wrapped', wrapped())
print('wrapped ref', wrapped_ref()())

Hopefully I'm not doing something blatantly wrong here; to be honest I'm still in the process of wrapping my head around this library lol

[GrahamDumpleton]

Do you have a practical use case for where weakref.WeakMethod would be used? Am curious, but also may help to understand what the purpose of it is.

[p7g]

For sure, though this might be a little bit niche:

I'm in the process of wrapping Django signals (for tracing purposes), which use a WeakMethod to hold onto references to bound functions (here). For this project I am also using django-filter, which registers a bound function as a signal handler here.

Tangentially related: I may also make a PR to that library to pass weak=False, since there's no real need for a weak reference there.

[GrahamDumpleton]

First observation is that the implementation of WeakMethod is arguably wrong. This is because it doesn't correctly apply the descriptor protocol to rebind the function to the instance when it calls it. Instead, it creates a new instance of the function from its type object and binds it by passing in the object as argument.

The implementation is:

    def __call__(self):
        obj = super().__call__()
        func = self._func_ref()
        if obj is None or func is None:
            return None
        return self._meth_type(func, obj)

I believe this should be:

    def __call__(self):
        obj = super().__call__()
        func = self._func_ref()
        if obj is None or func is None:
            return None
        return func.__get__(obj)

So on first look I believe that is an issue with the Python standard and a bug report should be raised.

Even were that correct, the next issue is that the code in WeakMethod does:

    def __new__(cls, meth, callback=None):
        try:
            obj = meth.__self__
            func = meth.__func__
        except AttributeError:
            raise TypeError("argument should be a bound method, not {}"
                            .format(type(meth))) from None

So it effectively unbinds or unrolls the bound method to extract the original method and the object it was bound to. Right now with the wrapt function wrapper, this results in the throwing away of the wrapper function as __func__ would return the original wrapped function. So even were the code above fixed, it would only end up rebinding the original function still, and would not use the wrapper.

It is quite possible that the wrapt function wrapper should override __func__ so that it returns an unbound version of the function wrapper so that rebinding could work and thus the wrapper would still be called, but even if it did that, it wouldn't work because of the problem above in the standard library. It is also debatable whether the result would be what is wanted anyway as far as weak method is concerned.

So that is sort of dead end. The next thing to do is look at the Django function where it is all been used. This contains:

        if weak:
            ref = weakref.ref
            receiver_object = receiver
            # Check for bound methods
            if hasattr(receiver, '__self__') and hasattr(receiver, '__func__'):
                ref = weakref.WeakMethod
                receiver_object = receiver.__self__
            receiver = ref(receiver)
            weakref.finalize(receiver_object, self._remove_receiver)

The problem here is the check of __self__ and __func__ as the wrapt function wrapper instance will show them as being present, but the issue is whether a weak method is even need needed in this case and whether the way the function wrapper works negates it.

One solution may therefore be to monkey patch the Signal.connect() method of Django and have the wrapper intercept the receiver argument as see whether it is a wrapt function wrapper and if it is, pass it through wrapped in a further wrapt object project which override __self__ property and have it raise AttributeError so that hasattr() thinks the function doesn't exist. This way it skip the weak method and use a normal weak reference. Whether that will work still not sure.

Anyway, that is all I can suggest for now, but will think about it some more.

[p7g]

Thanks for the thorough writeup!

I think your workaround should work well for my use-case, thanks 👍