Skip to content
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

Fix for issue #407: Prevent crashes due to unbounded growth. #721

Merged

Conversation

navarchus
Copy link
Contributor

@navarchus navarchus commented Aug 20, 2024

Prevent crashes due to unbounded growth of a few items.

Fixes #407.

…ble setting for max number of iotas on the stack, default is 2048 iotas.
@navarchus navarchus changed the title Prevent crashes due to unbounded growth. Fix for issue #407: Prevent crashes due to unbounded growth. Aug 20, 2024
@navarchus
Copy link
Contributor Author

Special thanks to @walksanatora for helping me learn the repo

@vgskye
Copy link
Contributor

vgskye commented Aug 20, 2024

@navarchus
Copy link
Contributor Author

navarchus commented Aug 20, 2024

shouldn't this use the limit at https://github.com/FallingColors/HexMod/blob/main/Common/src/main/java/at/petrak/hexcasting/common/lib/hex/HexIotaTypes.java#L26?

I believe this is a different enough scenario, as the bug in question is about a spell which continually adds to a list whereas the constants you reference seem to be more related to trying to limit the max size of a spell before it is cast.

@vgskye
Copy link
Contributor

vgskye commented Aug 20, 2024

shouldn't this use the limit at main/Common/src/main/java/at/petrak/hexcasting/common/lib/hex/HexIotaTypes.java#L26?

I believe this is a different enough scenario, as the bug in question is about a spell which continually adds to a list whereas these constants seem to be more related to trying to limit the max size of a spell before it is cast.

I believe the MAX_SERIALIZATION_TOTAL limit is commonly known as iota limit or serialization limit, which.... limits hexes in much the same way as this pull in practice, so I feel like treating this limit as an extension of that is reasonable.
(also it'd let you reuse IotaType.isTooLargeToSerialize for checking)

Copy link
Member

@object-Object object-Object left a comment

Choose a reason for hiding this comment

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

Looks good. It would be nice if the error message showed the pattern that caused it, but I'll do that myself later. Thanks for the PR!

@object-Object object-Object merged commit a763cdf into FallingColors:main Aug 20, 2024
3 checks passed
@navarchus navarchus deleted the navarchus-fix-no-stack-max branch August 21, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Crash by repeatedly doubling list size and running out of memory
3 participants