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

[struct_pack][bugfix] fix broken container size cause program crash #509

Merged
merged 3 commits into from
Dec 1, 2023

Conversation

poor-circle
Copy link
Collaborator

@poor-circle poor-circle commented Nov 29, 2023

… much memory

Why

The container size, now will check before read. But for performance, we will resize some container before check buffer, which may cause OOM/allocate too much memory.

What is changing

  1. Check the size if possible before read. (such as memory stream, for user-defined stream, it should add a member function bool check(std::size_t sz);
  2. If we can't check it, now container will pre-allocate at most 1M memory, and if deserialize failed, it will try to de-allocate it by call resize & shrink_to_fit(if possible).
  3. fix some overflow bug when the container size is very big.

Example

@poor-circle poor-circle changed the title [struct_pack][bufix] fix broken container size cause program crash/allocate too much memory [struct_pack][bugfix] fix broken container size cause program crash/allocate too much memory Nov 29, 2023
@poor-circle poor-circle changed the title [struct_pack][bugfix] fix broken container size cause program crash/allocate too much memory [struct_pack][bugfix] fix broken container size cause program crash Nov 29, 2023
Copy link
Collaborator

@qicosmos qicosmos left a comment

Choose a reason for hiding this comment

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

LGTM

@poor-circle poor-circle merged commit 63c3bf5 into alibaba:main Dec 1, 2023
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants