-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[config] protect is_serializable
against circular references
#12196
[config] protect is_serializable
against circular references
#12196
Conversation
Forgot to add a CHANGES entry but I'll do it next week (I'm leaving for today). |
I may also add a simple test for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @picnixz, at a quick high-level look I think this looks all good
I'm not entirely confident that the recursive guard based on the Now, I will not make it entirely foolproof because, as the name indicates, I don't want to protect against fools too much. |
I'm not entirely sure of the coverage since it's very hard to check things or generate examples of "broken" configurations that would be meaningful.
Note that it is not possible to test equality of objects with circular references using
==
(thus, we cannot test equality of objects with circular references after pickle+unpickle since the references are not the same).It does not fix #11752 because the recursion error occurs in the
__instancecheck__
and I was not given anything to reproduce (so I cannot say for sure that it was directly related to that or if the recursion error happened at that call site). However, we do have an issue with circular objects (if any).