functools.cache on property accessors leaks instances and returns wrong values across same-id objects

[NandaScott]

Problem

Three property accessors are decorated with @property over @functools.cache:

• scrython/base_mixins.py:17 — ScryfallListMixin.data
• scrython/cards/cards_mixins.py:168 — GameplayFieldsMixin.all_parts
• scrython/cards/cards_mixins.py:183 — GameplayFieldsMixin.card_faces

Decorators apply bottom-up, so cache wraps the underlying function and property wraps the cached function. That produces one unbounded cache dict living on the function object, shared by every instance of every class that inherits the mixin, keyed on self via __hash__ / __eq__.

Two distinct problems follow.

1. Instances are retained for the lifetime of the process

The cache holds a strong reference to self as its key and never evicts. Any object whose data, all_parts, or card_faces was read can never be garbage collected.

import gc, weakref
from scrython.base_mixins import ScryfallListMixin

class L(ScryfallListMixin):
    list_data_type = None
    def __init__(self, d): self._scryfall_data = d

obj = L({"data": [1, 2], "has_more": False})
ref = weakref.ref(obj)
obj.data                 # populates the cache
del obj; gc.collect()
print(ref() is not None)  # True — leaked

obj2 = L({"data": [3], "has_more": False})
ref2 = weakref.ref(obj2)  # .data never touched
del obj2; gc.collect()
print(ref2() is not None)  # False — collected normally

Code that paginates many search results, or builds many card objects in a loop, grows without bound.

2. Objects sharing a Scryfall ID return each other's cached values

ScrythonRequestHandler.__eq__ / __hash__ (scrython/base.py:372, :404) and cards.Object.__eq__ / __hash__ (scrython/cards/cards.py:51, :72) compare and hash by Scryfall id, not identity. functools.cache looks keys up by hash and equality, so two distinct objects carrying the same id collide on one cache entry and the second one silently receives the first one's computed value.

import json
from scrython.cards import Object

fix = json.load(open("tests/fixtures/cards/named.json"))

def make(uid, card_faces=None):
    data = dict(fix)
    data["id"] = uid
    if card_faces:
        data["card_faces"] = card_faces
    return Object.from_dict(data)

first = make("uid-A", card_faces=[{"name": "Front"}, {"name": "Back"}])
second = make("uid-A")  # same id, genuinely has no card_faces

print([f._scryfall_data["name"] for f in first.card_faces])
# ['Front', 'Back']
print("card_faces" in second._scryfall_data)
# False
print([f._scryfall_data["name"] for f in second.card_faces])
# ['Front', 'Back']  — wrong, leaked from `first`

The reverse order fails the same way: when the faceless object is read first, an object that does have card_faces returns None. Objects with no id key (list and catalog envelopes) fall back to identity in both __eq__ and __hash__, so they are not affected by this second problem — only by the leak.

This is reachable without touching private state: fetching the same card through two endpoints (Named and ById), or rehydrating a stored dict alongside a live fetch, produces two distinct objects with one id.

Suggested fix

Replace @property + @cache with functools.cached_property on all three accessors. It stores the computed value in the instance's own __dict__, so the value dies with the instance and no cross-object lookup is possible. Per-instance caching and the stable return identity on repeated access are both preserved, so this is not an API change.

Worth checking during the fix whether these accessors need caching at all — data with list_data_type=None is a plain dict read.

Acceptance criteria

• No accessor uses @property over functools.cache.
• An instance whose cached accessor was read is garbage collected once the caller drops it (weakref regression test).
• Two distinct objects with the same Scryfall id and different underlying data return their own values from card_faces and all_parts (regression test).
• Repeated access on one instance still returns the identical object.
• pytest green.

Notes

Found while reviewing #203. Out of scope there: PRD #180 bars production changes to the mixins.

Unrelated docstring inaccuracy in the same area, cheap to fix alongside: ScrythonRequestHandler.__hash__ documents "hash of class name if no ID available" but actually returns hash(id(self)).