fix memory leak with iterable validation

[davidhewitt]

Change Summary

Fixes a memory leak when validating an iterable. I'll try to work out a test before this merges.

The general problem was that the Iterable field gets populated with a ValidatorIterable, which is a huge stateful object which itself contained model's __dict__, which eventually includes the ValidatorIterable itself. This created a cycle which was opaque to the garbage collector.

Implementing the __traverse__ for the ValidatorIterable object fixes the problem, but to be honest, it feels like a big design wart that we have such a big stateful iterable here. Arguably, it's a massive footgun.

For example, the following doesn't work, because the elements stored is the list iterator wrapped up in the ValidatorIterable.

from collections.abc import Iterable
from pydantic import BaseModel
import pydantic

class A(BaseModel):
    elements: Iterable[int] | None

# nope, elements is a ValidatorIterator
assert A(elements=[1, 2, 3]).elements == [1, 2, 3]

Related issue number

Fixes pydantic/pydantic#9243

Checklist

• Unit tests for the changes exist
• Documentation reflects the changes where applicable
• Pydantic tests pass with this pydantic-core (except for expected changes)
• My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

[davidhewitt]

Speaking with @samuelcolvin it seems like revisiting the Iterable type would be a good thing for V3.

e.g.

• could the original input list be elements here (or a copy) - for example Sequence returns a list
• the validation of the elements is currently done lazily, should it be done eagerly?

Probably the functionality we have here is useful to some people, so we should keep it but move it to be something deliberate. For most people it's likely needless complexity.

[codspeed-hq]

CodSpeed Performance Report

Merging #1271 will not alter performance

_{Comparing dh/iterable-leak (466dafc) with main (4adf47f)}

Summary

✅ 155 untouched benchmarks

[afdw]

Speaking with @samuelcolvin it seems like revisiting the Iterable type would be a good thing for V3.

I think the Iterable[T] support is indeed not done right. There is at least one very important problem: it actually converts an Iterable[T] (something that can give rise to an Iterator[T]) into an Iterator[T] itself, which is not really correct. Consider the following:

from collections.abc import Iterable
from pydantic import BaseModel, ConfigDict


class Trace(BaseModel):
    model_config = ConfigDict(frozen=True)

    elements: Iterable[int]

t = Trace(elements=[1, 2, 3])
print(list(t.elements)) # outputs [1, 2, 3]
print(list(t.elements)) # outputs []

So validating an immutable object results in a mutable one, which seems wrong to me. What it should have done was to keep being an Iterable[T], and only construct an Iterator[T] when requested, and each time a different one.

This would also solve the problem of storing a huge stateful object in the model, as only a thin wrapper around the original Iterable[T] provided to the validation function would be saved.

• the validation of the elements is currently done lazily, should it be done eagerly?

For me eager validation would not work at all. My use case for Iterable[T] is validating huge amounts of data as a stream. This means that trying to do it eagerly would exhaust system's memory very fast, as it would try to store all the results immediately. Where as using Iterable[T] allows to have validation on the fly, while I am iterating it, which is what I want.

Also, if it would be done eagerly, how it would be different from list[T]? Could not people just use a list instead if that is what they need?

[davidhewitt]

All great questions. Let's move the redesign question to pydantic/pydantic#9266 (and we have no concrete date for V3 yet, so this would just be collecting thoughts for now).

I'll consider whether I can come up with a test case here later.

[adriangb]

Does assert list(A(elements=[1, 2, 3]).elements) == [1, 2, 3] work?

[davidhewitt]

Yes (and this PR hasn't changed that)

[adriangb]

I just wanted to point out that the behavior seems reasonable to me in that case.

The main thing I would complain about is that I would want it to eagerly validate any finite inputs. So if I give it a list it eagerly evaluates it. If I give it a generator I'd be either okay with or maybe even prefer that it be lazy (maybe an annotation option would be good); perhaps a beartype like solution where it eagerly validates the first N would be good? (this is all a side note, not suggesting any changes for this PR)

[billou57]

are you able to identify in which version this bug was introduced ?

[davidhewitt]

I think this probably existed since 2.0.