[parijat1222q]

Fixes #364

With @wrapt.lru_cache above @classmethod, the shared cache currently wraps the first bound method. Later calls through a subclass therefore receive the first caller's class, even for arguments not already cached.

Cache the unbound function and pass the bound class as part of the cache key. This keeps the documented single shared cache and maxsize budget, while preserving cls for each caller. Add a regression covering both initial call orders, inherited instance access, fresh arguments, shared statistics and cross-class cache clearing, plus a change note. This change is limited to the documented decorator order.

Validation:

• The new regression fails on the unchanged base with the C extension enabled and disabled, then passes with the fix.
• All 27 full-suite configurations passed using the Justfile test-version recipes: Python 3.9-3.15 plus 3.14t/3.15t, each with extensions absent, enabled and disabled at runtime. Python 3.14's three runs report 1417 passed / 2 skipped, 1419 passed, and 1417 passed / 2 skipped.
• just test-mypy passed for all seven target versions with the pinned mypy versions. Black on both changed Python files and git diff --check passed.

These are local results. Hosted CI and maintainer review are not claimed.

[GrahamDumpleton]

Thanks for the report and the detailed reproducer. The bug is real: the
shared cache for a class method is built from whichever bound method
happens to be seen first, so the class from that first lookup is baked
into the cache and every later call through a subclass gets the wrong
cls. Your analysis of the cause is correct.

I don't want to take this fix as it stands though, because of the way it
obtains the function to cache. Reaching through __wrapped__.__func__
gets the innermost raw function, which skips anything wrapped between
lru_cache and the classmethod descriptor. For example:

@wrapt.decorator
def audit(wrapped, instance, args, kwargs):
    log.append((instance, args))
    return wrapped(*args, **kwargs)

class Base:
    @wrapt.lru_cache
    @audit
    @classmethod
    def identify(cls, value):
        return cls.__name__, value

class Child(Base):
    pass

On current develop the cls reported is wrong, but audit is called
on every cache miss. With this change the cls is right, but audit is
never called at all, because the bound __wrapped__ here is a
BoundFunctionWrapper whose __func__ proxies straight through to the
original function. The intermediate decorator is silently dropped. The
rest of wrapt treats __wrapped__ as the thing to call and never looks
past it, and lru_cache needs to do the same.

I have a different approach in mind which keeps the single shared cache,
keys it by the bound class as functools.lru_cache does, and still
calls through __wrapped__ so stacked decorators keep working. I'm
testing that now. I'll land it as my own change on develop along with
a test for the stacked decorator case, and will close this pull request
in favour of that. The change note will credit you for the report and
the regression test, which I'll keep.

[parijat1222q]

Thanks for explaining the stacked-decorator case. Reaching through __func__ loses the intermediate wrapper, so calling through __wrapped__ is important here. Thanks for keeping the regression test and offering to credit the report.

[GrahamDumpleton]

A follow up on where I've got to with this.

I first tried a variant of your approach which kept the single shared
cache and keyed it by the bound class, but without reaching through
__func__. The bound method itself was used as the leading cache key,
since bound methods compare equal and hash the same when __self__ and
__func__ match, and the cache called through __wrapped__ so stacked
decorators still ran. That fixed the reported problem, but it has the
same garbage collection issue that functools.lru_cache has when used
under classmethod: the cache on the base class holds a strong
reference to every subclass that has ever called the method, for as
long as its entries remain.

class Base:
    @wrapt.lru_cache
    @classmethod
    def method(cls, value):
        return value

Child = type("Child", (Base,), {})
Child.method(1)
ref = weakref.ref(Child)
del Child
gc.collect()
ref() is not None   # True, Child is pinned by Base's cache

With maxsize=None nothing is ever evicted, so dynamically created
subclasses would accumulate for the life of the base class. The
stdlib behaves identically, but wrapt.lru_cache exists precisely to
avoid that category of problem for instance methods, so I don't want to
accept it for class methods just because the stdlib does.

What I'm currently testing instead treats class methods the same way as
instance methods. Each class the method is bound to gets its own cache,
stored as an attribute in that class's own __dict__. For an inherited
class method the bound class is the subclass, so:

class Base:
    @wrapt.lru_cache(maxsize=32)
    @classmethod
    def method(cls, value):
        return cls, value

class Child(Base):
    pass

Base.method(1)     # (Base, 1)
Child.method(1)    # (Child, 1)
Child.method(2)    # (Child, 2)
Child().method(1)  # (Child, 1), cache hit

Base.method.cache_info()
# CacheInfo(hits=0, misses=1, maxsize=32, currsize=1)
Child.method.cache_info()
# CacheInfo(hits=1, misses=2, maxsize=32, currsize=2)

Child.method.cache_clear()   # clears only Child's cache

What this means in practice:

• The class method always receives the class it was called through,
  which fixes the bug you reported.
• Each class in the hierarchy has its own cache and its own full
  maxsize budget, rather than sharing one across all subclasses.
• cache_info(), cache_clear() and cache_parameters() accessed via
  a class operate on that class's cache only.
• The cache is collected along with the class, so the example above
  with the dynamically created subclass now returns None from the
  weak reference.
• Decorators stacked between lru_cache and classmethod are still
  called on every miss, since the cache calls through __wrapped__.

The lookup has to read the class's own __dict__ rather than use
normal attribute access, otherwise a subclass would find and share the
base class cache and we'd be back to the original problem.

The cost is that the documentation currently promises a single shared
cache for class methods, and this changes that. I'm still weighing
whether a per class cache is the right end state, or whether there is a
way to keep a shared cache without the retention problem. The aim is
sensible behaviour for wrapt rather than fidelity to the stdlib, so the
shared cache isn't sacred, but I want to be sure before committing to
a documented behaviour change. Your regression test is kept in either
case, adjusted for whichever semantics land.

[parijat1222q]

I tried a small weak-class-key prototype with a shared cache. It preserves the shared budget and calls through the decorator chain, but only solves the direct key reference: a cached result such as (cls, value), or an argument that is an instance of cls, still keeps the subclass alive. A per-class cache avoids that external root. So weak keys alone don't seem sufficient to preserve the current shared-cache semantics without retention.

[parijat1222q]

A possible middle ground is class-owned entries with a shared, weakly held LRU order. Each class keeps its strong keys/results, while one coordinator manages the global maxsize, statistics and clearing. It must use each class's own __dict__, not an inherited entry map.

This runnable example shows the ownership part only, not a complete cache:

import gc
import weakref
from collections import OrderedDict

class Entry:
    def __init__(self, value):
        self.value = value

lru = OrderedDict()  # order only; entries are weakly held

class Base:
    pass

Child = type('Child', (Base,), {})
entry = Entry((Child, 1))
Child._entries = {1: entry}
key = object()
lru[key] = weakref.ref(entry, lambda ref: lru.pop(key, None))
ref = weakref.ref(Child)
del entry, Child
gc.collect()
assert ref() is None
assert not lru

In the fuller prototype, hits move an entry's token to the end of the shared order; eviction removes the oldest entry from its owner's map. The coordinator holds only weak entry references, and entries hold weak owner references. Misses call through the full bound wrapper, not __func__.

Focused tests preserve one shared budget, result identity and the decorator chain, while allowing otherwise-unreachable subclasses to collect even when their own results refer back to them. Another live class caching Child, or a caller retaining a class-containing result, still legitimately retains it.

The cost is a custom LRU and extra bookkeeping. This is a proof of concept, not a production-ready patch: full wrapt integration, reentrant keys and broader concurrency testing remain open. The simpler per-class design may still be the better tradeoff.

[GrahamDumpleton]

Having thought about this more, I've decided the per class cache is the
right end state rather than a workaround, and here is why.

Consider what Python already does when a class method stores anything
through cls:

class A:
    @classmethod
    def f(cls, v):
        cls.k = v

class B(A):
    pass

A.f(1)
B.f(2)

print(vars(A)["k"])   # 1
print(vars(B)["k"])   # 2

The value written by B.f(2) lands on B, even though f is defined
on A. A class method is bound to the class it is called through, and
state it keeps lives on that class. A cache is nothing more than state
the method accumulates, so the natural home for it is the same place
Python puts everything else the method stores. One cache on A shared
by every subclass is the design that needs justifying, not the per
class one.

The same conclusion comes from treating a class method as what it is:
an instance method whose instance is a class, with cls in the role
of self. wrapt.lru_cache already refuses to share a cache across
instances, because a shared cache makes every instance compete for one
maxsize budget and keeps every instance alive through the keys.
Replace "instance" with "class" and both problems are still there. The
third reason, hashability, is the only one that drops away. If the
answer for self is to store the cache on self, the answer for cls
is to store it on cls.

It also lines up with correctness. If B overrides a class attribute
that f reads, A.f(1) and B.f(1) are entitled to return different
results, so any correct cache has to keep their entries apart. Keying
a shared cache by class achieves that by bookkeeping. Giving each class
its own cache achieves it because the entries never meet in the first
place.

The visible consequence is that A.f.cache_clear() no longer empties
the cache for B.f. I'm comfortable with that, since it is exactly how
A.k and B.k behave in the first example, and the documentation now
describes it. The goal for wrapt.lru_cache is behaviour that makes
sense for methods, not fidelity to functools.lru_cache, whose
handling of methods is the reason this decorator exists.

[parijat1222q]

Would you like me to update this PR to the per-class cache design, or are you already handling the implementation?

[GrahamDumpleton]

I already have it implemented. Had just been thinking it through, plus had to go out and do some running around.

[GrahamDumpleton]

Is on develop now so you can see how that works for you.

[parijat1222q]

Thanks, I tested the develop change locally on Python 3.14. The lru_cache tests pass with and without the C extension, including the inherited-class regression. Thanks for keeping the test and crediting the report.