KeyError in GenericModel.__parameterized_bases__.build_base_model

[crazyscientist]

Bug

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.9.1
            pydantic compiled: True
                 install path: /home/andi/virtualenvs/aimaas/lib/python3.9/site-packages/pydantic
               python version: 3.9.12 (main, Apr 18 2022, 22:40:46)  [GCC 9.4.0]
                     platform: Linux-5.4.0-113-generic-x86_64-with-glibc2.31
     optional deps. installed: ['dotenv', 'typing-extensions']

I'm afraid reproduction of this issue is a bit tricky as I was not able to reproduce it in a production scenario nor with a small code snippet. So far, I only noticed this in the test suite of this project, which contains multiple sub-modules for tests; but only tests in two can fail, i.e. when the entire testsuite runs, they fail; if I explicitly only run tests from these modules, they pass.

This behavior probably also indicates a problem within the testsuite, which I intend to look for, but the problem does not occur with version 1.9.0 of pydantic. So, I figured it's worth sharing.

In the project, we dynamically define API routes for FastAPI for dynamically generated models (i.e. model definitions are stored in DB), i.e. the models and routes get generated/updated at start-up or when something changes in the DB. The callback function used for this looks something like this:

def route_get_entities(router: APIRouter, schema: Schema):
    # this is the callback
    factory = EntityModelFactory()
    filter_model = _filters_request_model(schema=schema)

    @router.get(
        f'/{schema.slug}',
        response_model=Page[factory(schema=schema, variant=ModelVariant.GET)],
        response_model_exclude_unset=True
    )
    def get_entities(
        filters: filter_model = Depends(),
        order_by: str = Query('name', description=''),
        ascending: bool = Query(True, description=''),
        db: Session = Depends(get_db),
        params: Params = Depends()
    ):
        # the actual API route
        ...

The exception raised in the testsuite looks like this:

_____________________________________ ERROR at setup of TestRouteGetEntityChanges.test_raise_on_change_doesnt_exist ______________________________________

dbsession = <sqlalchemy.orm.session.Session object at 0x7fca28575580>

    @pytest.fixture
    def client(dbsession):
>       app = create_app(session=dbsession)

backend/tests/conftest.py:338: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
backend/__init__.py:53: in create_app
    load_dynamic_routes(db=session, app=app)
backend/__init__.py:40: in load_dynamic_routes
    create_dynamic_router(schema=schema, app=app)
backend/dynamic_routes.py:341: in create_dynamic_router
    route_get_entities(router=router, schema=schema)
backend/dynamic_routes.py:124: in route_get_entities
    response_model=Page[factory(schema=schema, variant=ModelVariant.GET)],
../../virtualenvs/aimaas/lib/python3.9/site-packages/pydantic/generics.py:98: in __class_getitem__
    __base__=(cls,) + tuple(cls.__parameterized_bases__(typevars_map)),
../../virtualenvs/aimaas/lib/python3.9/site-packages/pydantic/generics.py:224: in __parameterized_bases__
    yield from build_base_model(base_model, typevars_map)
../../virtualenvs/aimaas/lib/python3.9/site-packages/pydantic/generics.py:190: in build_base_model
    parameterized_base = base_model.__class_getitem__(base_parameters)
../../virtualenvs/aimaas/lib/python3.9/site-packages/pydantic/generics.py:98: in __class_getitem__
    __base__=(cls,) + tuple(cls.__parameterized_bases__(typevars_map)),
../../virtualenvs/aimaas/lib/python3.9/site-packages/pydantic/generics.py:224: in __parameterized_bases__
    yield from build_base_model(base_model, typevars_map)
../../virtualenvs/aimaas/lib/python3.9/site-packages/pydantic/generics.py:185: in build_base_model
    base_parameters = tuple([mapped_types[param] for param in base_model.__parameters__])
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

.0 = <tuple_iterator object at 0x7fca2344e370>

>   base_parameters = tuple([mapped_types[param] for param in base_model.__parameters__])
E   KeyError: ~T

../../virtualenvs/aimaas/lib/python3.9/site-packages/pydantic/generics.py:185: KeyError

I checked the contents of mapped_types and base_model.__parameters__ in the line where the exception gets raised:

>>> mapped_types
{~T: <class 'pydantic.main.PersonGet'>}
>>> a = list(mapped_types.keys())[0]
>>> a
~T
>>> b = base_model.__parameters__
>>> b[0]
~T
>>> a is b[0]
False
>>> type(a)
<class 'typing.TypeVar'>
>>> type(b[0])
<class 'typing.TypeVar'>

It looks like the TypeVars, which are expected to be identical, no longer are. 🤔

[crazyscientist]

I was not able to create a short example for reproducibility, but I managed to identify the changes that "broke" our testsuite: #4083

If I revert both changes to pydantic/generics.py, the errors in our testsuite disappear.

That might explain, why it's not easy to create a simple snippet for reproducibility, right? One needs to manage to exceed the limit...

[samuelcolvin]

Sorry for slow reply, yes makes sense.

I don't want to change the limits, but we should fail in a more transparent way.

[crazyscientist]

I also added some debug messages in pydantic/generics.py to better understand what's happening.

I noticed for my case, that the caches _generic_types_cache and _assigned_parameters grow with each unittest, when the FastAPI app is re-created (in a fixture explicitly via create_model). It looks to me, like the retrieval from these caches is broken for dynamic models and/or TypeVars:

The testsuite fixture defines a limited number of distinct models (including static and dynamic ones, in total 23), but whenever the application is rebuilt for the next test case, the models get rebuilt and added to the cache as the keys do not match. To be exact, _generic_types_cache very quickly grows to the specific limit, whereas _assigned_parameters goes up only to about 600.

To the best of my knowledge, this cache problem (maybe the reason for the leak mentioned in #3829?) was also true for v1.9.0, but for some reasonit did not cause a KeyError.

I don't want to change the limits

Yes, if my understanding of the observed problem is correct, changing the limit makes no difference.

we should fail in a more transparent way.

Given the specific error I observed, I'm wondering if it would make more sense to allow a margin of error when comparing two different TypeVars that have the same name?

But that would probably only count as a workaround.

Maybe I should raise an issue with the developers of fastapi_pagination to not use TypeVars or raise an issue with the developers of Python so that the comparison of TypeVars changes:

>>> import typing
>>> t = typing.TypeVar("T")
>>> s = typing.TypeVar("T")
>>> t == s
False
>>> t is s
False

[MarkusSintonen]

Hello!

We are also getting hit by this issue. Im not sure is it related to TypeVar handling in our case as @crazyscientist mentioned. But its related to generics with parametrized bases. The issue is actually very easy to reproduce:

from typing import Generic, TypeVar
from weakref import WeakKeyDictionary, WeakValueDictionary

import pydantic
from pydantic import create_model
from pydantic.generics import GenericModel

# Uncomment to fix the issue
# pydantic.generics._generic_types_cache = WeakValueDictionary(pydantic.generics._generic_types_cache)
# pydantic.generics._assigned_parameters = WeakKeyDictionary(pydantic.generics._assigned_parameters)

T = TypeVar("T")
C = TypeVar("C")

class A(GenericModel, Generic[T, C]): pass
class B(A[int, C], GenericModel, Generic[C]): pass

models = [create_model(f"M{i}") for i in range(350)]
for m in models:
    B[m]  # Will crash with "KeyError: ~T"

So essentially generics with parametrized bases are broken due to the PR changing to use LimitedDicts by @samuelcolvin. As far as I understand, it seems using LimitedDict is not correct as it will arbitrarily break usage of generics with parametrized bases.

The example I gave is actually not something we use. Its just to repro the issue. The count of models there can be actually much lower depending on complexity of the application and usage of generics and parametrized bases.

I also gave a fix proposal above so moving to use weak dictionaries to handle any leakage with dynamic model usage. We actually need to now apply the above fix to our code as otherwise it can crash with KeyErrors.

We are using Pydantic 1.10.4.

[samuelcolvin]

Thanks for your detailed explanation @MarkusSintonen, we'll review your PR.

[MarkusSintonen]

Thanks for your detailed explanation @MarkusSintonen, we'll review your PR.

Great thanks! The PR should be now ready for review

[adriangb]

THere's been several PRs addressing memory leaks with generics (including the one above, which is merged). I believe this is no longer an issue, especially for Pydantic V2. Please re-open if it still is.

[crazyscientist]

When I last checked with v1.10.4, the problem still existed, but when I checked today with v1.10.8 the problem seems to be fixed.

Thank you 👍

[lig]

@crazyscientist BTW, I'd still recommend using a constraint pydantic<2 in preparation for Pydantic v2 release. There will be some significant breaking changes in v2. I would strongly recommend avoiding automatic upgrades from v1 to v2.