Skip to content

gh-146059: Call fast_save_leave() in pickle save_frozenset()#146173

Open
vstinner wants to merge 3 commits intopython:mainfrom
vstinner:pickle_fast
Open

gh-146059: Call fast_save_leave() in pickle save_frozenset()#146173
vstinner wants to merge 3 commits intopython:mainfrom
vstinner:pickle_fast

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Mar 19, 2026

Copy link
Member Author

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buf = io.BytesIO()
pickler = self.pickler(buf, protocol=proto)
# Enable fast mode (disables memo, enables cycle detection)
pickler.fast = 1
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be also worth it to test non-fast mode? The bug only triggers in fast mode.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add also tests for list, set, dict, tuple and frozendict.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this mechanism is used to guard against deeply nested structures, so it would be worth to test deeply nested structures.

There are sets of tests for recursive structures. We can use similar tests for limited but deep recursion.

# Enable fast mode (disables memo, enables cycle detection)
pickler.fast = 1

if tested_type == frozenset:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More Pythonic way would be to make tested_type and data parameters and call the function with different arguments.

@vstinner
Copy link
Member Author

@serhiy-storchaka: Please review the updated PR. Is it what you expected?

Also, this mechanism is used to guard against deeply nested structures, so it would be worth to test deeply nested structures.

I added deep_nested_struct() tests for the 6 types.

More Pythonic way would be to make tested_type and data parameters and call the function with different arguments.

Good idea, I rewrote tests like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants