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 a memory leak in AlignedVector. #13005
Conversation
resize(0); | ||
resize_fast(vec.used_elements_end - vec.elements.get()); | ||
reserve(new_size); |
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.
The bug is here: When we call resize_fast()
, we allocate memory and default construct objects in that memory. But in the line below (AlignedVectorCopyConstruct
) we copy-construct other objects in that same memory location, forgetting to call the destructor on what we had done before.
The solution is to not initialize the memory we allocate -- that's what reserve()
does in contrast to resize*()
.
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.
For reference, on second look, I see that in essence the same code I just wrote here is also what's in the copy constructor just a few lines up.
@elauksap FYI. |
That's brilliant, thanks a lot! |
@elauksap Could you try out this patch without #12995. |
I would vote for keeping the patch, it is a nice use of compile-time knowledge and hence more direct in my opinion. |
Fixes #12998. As pointed out in #12993.
/rebuild