Preserve descriptor binding in weak proxies for decorated callables - #356
GrahamDumpleton merged 4 commits into
Conversation
|
Do you have example code which resulted in you finding this so I have a better idea of use cases that cause a problem. Thanks. |
|
Thanks for taking a look. I found this while reviewing import wrapt
@wrapt.decorator
def traced(wrapped, instance, args, kwargs):
return wrapped(*args, **kwargs)
@traced
def on_event(value):
return value + 1
class Handler:
@traced
@classmethod
def on_event(cls, value):
return cls.__name__, value
class Child(Handler):
pass
for callback in (on_event, Handler.on_event, Child.on_event):
try:
registered = wrapt.WeakFunctionProxy(callback)
print(registered(41))
except TypeError as error:
print(type(error).__name__ + ': ' + str(error))I ran this against the base commit ( The free function has no bound instance. The class-accessed methods likewise have a wrapper instance of I also investigated the Python 3.13 free-threaded CI failure. The new owner-lifetime test incorrectly assumed classes could be collected there after threads had started. I reproduced the same retention with a plain class and no wrapt import; it matches CPython 3.13's documented class immortalization. The follow-up skips only that owner-collection test on free-threaded 3.13. The callable binding and function-expiration tests still run there, and the owner-collection test remains active on other interpreter configurations. No runtime implementation change was needed for this CI failure. Local verification after the test correction: the full suite passes with pure Python, C extensions enabled, and C extensions disabled at runtime on each of Python 3.13t, 3.13, and 3.14t. For 3.13t, the three results were 1,265 passed / 9 skipped, 1,267 passed / 7 skipped, and 1,265 passed / 9 skipped. The 3.13t run used the pinned mypy 1.20.1 from a separate standard-Python environment because its librt dependency could not build against my local 3.13t interpreter; the runtime tests themselves ran under 3.13t. The 3.13 and 3.14t runs used the normal Justfile recipes. The correction is in commit 07db6cd. |
|
They have started saying that 3.13t should be effectively ignored since it was the experimental version. I already ignore it when doing local tests because of issues with it, although that may be macOS specific and related to shared library availability (can't remember). So I should perhaps consider just dropping 3.13t testing from the GitHub actions workflow. I'll decide that when I properly review the PR. Thanks for the report. |
The comments in weakrefs.py described the weak reference as being held only against the instance and the function, so extend them to cover the owner which is now also retained for function wrappers, and why it is needed. Add a release note under 2.4.2 for the fix.
…criptor-binding # Conflicts: # docs/changes.rst
|
Thanks for this, the analysis is right and the fix is the correct one. I confirmed that only avoiding the I've pushed one extra commit on top of your branch, with no changes to your code or tests:
I also merged Nothing else is needed from your side. Will merge once CI is through. |
WeakFunctionProxy raises TypeError for decorated free functions and class-accessed descriptors because their wrapper instance can be None. Simply avoiding weakref.ref(None) is insufficient: a descriptor accessed through a class still needs its original owner when rebound, including when inherited by a subclass.
Keep the instance optional and retain the descriptor owner through a weak reference. Rebind with that owner when calling the proxy, preserving classmethod, staticmethod, and unbound instance-method behavior without keeping the class alive.
Add regressions for decorated free functions, descriptor access through classes and instances (including subclasses), and garbage collection. The tests also check that the expiration callback runs once and expired calls raise ReferenceError. The new free-function and descriptor tests fail on the original code.
Validation: the full
just testmatrix passes on Linux across Python 3.9 through 3.15, including 3.14t and 3.15t. Each of the nine interpreter configurations runs the full suite with a pure-Python installation, the C extension enabled, and the C extension disabled at runtime. The targeted weak-proxy tests also pass on Windows/Python 3.12.