Skip to content

Commit 7edb41c

Browse files
committed
fix: stop keeping a model alive for the lifetime of the process
Subscribing a Path or a ScimFilter to a model, and resolving a path against one, both filed it in a module-level cache that never let go, so a server building a model per schema it discovers accumulated every one of them. The class cache holds its classes weakly, and the resolution cache moved onto the model, which is the only place a resolved attribute naming that model cannot outlive it.
1 parent 399d8e1 commit 7edb41c

3 files changed

Lines changed: 66 additions & 4 deletions

File tree

‎scim2_models/binding.py‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
from collections.abc import MutableMapping
12
from typing import TYPE_CHECKING
23
from typing import Any
4+
from weakref import WeakValueDictionary
35

46
from pydantic import GetCoreSchemaHandler
57
from pydantic import GetJsonSchemaHandler
@@ -12,9 +14,15 @@
1214
if TYPE_CHECKING:
1315
from .base import BaseModel
1416

15-
_BOUND_CLASSES: dict[tuple[type, tuple[type, ...]], type] = {}
17+
_BOUND_CLASSES: "MutableMapping[tuple[type, tuple[type, ...]], type]" = (
18+
WeakValueDictionary()
19+
)
1620
"""The classes subscription has already built, so that two subscriptions of the
17-
same resource types answer the same class."""
21+
same resource types answer the same class.
22+
23+
The classes are held weakly: one bound to a model built at runtime, as a server
24+
serving a schema it discovered does, would otherwise keep that model alive for
25+
as long as the process runs."""
1826

1927

2028
class _BoundToModels:

‎scim2_models/resolution.py‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
from dataclasses import dataclass
22
from dataclasses import replace
3-
from functools import lru_cache
43
from inspect import isclass
54
from typing import Any
65
from typing import cast
@@ -22,6 +21,9 @@
2221
# Python types that RFC7644 §3.4.2.2 forbids comparing with an ordering operator.
2322
_UNORDERABLE_TYPES = (bool, bytes)
2423

24+
_RESOLVED_ATTRS = "__scim_resolved_attrs__"
25+
"""The attribute a model keeps its resolved paths under."""
26+
2527

2628
def _unwrap_annotated(type_: Any) -> type | None:
2729
"""Strip the metadata of an :data:`~typing.Annotated` type.
@@ -189,7 +191,6 @@ def _target_model(
189191
raise PathNotFoundException(path=str(attr_path), field=attr_path.attr)
190192

191193

192-
@lru_cache(maxsize=1024)
193194
def resolve_attr_path(
194195
model: type[BaseModel], attr_path: AttrPath, *, strict: bool = True
195196
) -> ResolvedAttribute | None:
@@ -204,6 +205,26 @@ def resolve_attr_path(
204205
is safe since :class:`ResolvedAttribute` is immutable.
205206
:raises PathNotFoundException: If ``strict`` and the attribute is unknown.
206207
"""
208+
# The cache is held by the model rather than by this module, so that it
209+
# cannot outlive the model it describes: a resolved attribute names the
210+
# model it was resolved on, which a cache keyed by that model would keep
211+
# alive for as long as the process runs.
212+
cache: dict[tuple[AttrPath, bool], ResolvedAttribute | None] | None
213+
cache = model.__dict__.get(_RESOLVED_ATTRS)
214+
if cache is None:
215+
cache = {}
216+
setattr(model, _RESOLVED_ATTRS, cache)
217+
218+
key = (attr_path, strict)
219+
if key not in cache:
220+
cache[key] = _resolve_attr_path(model, attr_path, strict=strict)
221+
return cache[key]
222+
223+
224+
def _resolve_attr_path(
225+
model: type[BaseModel], attr_path: AttrPath, *, strict: bool
226+
) -> ResolvedAttribute | None:
227+
"""Bind an attribute path, without going through the cache of the model."""
207228
target = _target_model(model, attr_path, strict=strict)
208229
if target is None:
209230
return None

‎tests/test_dynamic_resources.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import datetime
2+
import gc
3+
import weakref
24
from typing import Union
35

46
from pydantic import Base64Bytes
@@ -10,6 +12,8 @@
1012
from scim2_models.annotations import Uniqueness
1113
from scim2_models.attributes import ComplexAttribute
1214
from scim2_models.context import Context
15+
from scim2_models.filters import ScimFilter
16+
from scim2_models.path import Path
1317
from scim2_models.reference import URI
1418
from scim2_models.reference import External
1519
from scim2_models.reference import Reference
@@ -2868,3 +2872,32 @@ def test_extensions_built_from_a_schema_know_their_attribute_urns():
28682872
"schemas": ["urn:example:2.0:Single", "urn:example:2.0:Ext"],
28692873
"urn:example:2.0:Ext": {"attr": "value"},
28702874
}
2875+
2876+
2877+
def test_a_model_built_at_runtime_is_collected_once_it_is_dropped():
2878+
"""A model discovered from a schema is not kept alive by what resolved against it.
2879+
2880+
A server serving the schemas of its tenants builds a model per schema, and
2881+
the classes a subscription answers, as well as the paths resolved against
2882+
them, used to hold every one of them for as long as the process ran.
2883+
"""
2884+
Model = Resource.from_schema(
2885+
_single_attribute_schema({"name": "label", "type": "string"})
2886+
)
2887+
reference = weakref.ref(Model)
2888+
2889+
# The assertions are made on values that do not name the model, since a
2890+
# resolved attribute does and pytest keeps the operands of an assertion.
2891+
resolved = Path[Model]("label").resolve() is not None
2892+
matched = ScimFilter[Model]('label eq "x"').match(Model(label="x"))
2893+
assert resolved
2894+
assert matched
2895+
2896+
del Model
2897+
2898+
# The first pass frees the validators pydantic built for the model, which
2899+
# is what leaves the model itself unreachable for the second one.
2900+
gc.collect()
2901+
gc.collect()
2902+
2903+
assert reference() is None

0 commit comments

Comments
 (0)