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

Fixing static_vector memory leak in validatePipelineBindingLayouts #55

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

loopunit
Copy link
Contributor

DeviceWrapper::validatePipelineBindingLayouts will leak memory allocated for bindingsPerLayout & duplicatesPerLayout's members due to the way static_vector's resize is implemented, which assumes that all std::array's elements are in an uninitialized state by default.

Changing static_vector to use assignment in resize instead of placement new/delete fixes the issue without adding a bunch of extra plumbing or running the destructor over each element in static_vector's constructor.

DeviceWrapper::validatePipelineBindingLayouts will leak memory allocated for bindingsPerLayout & duplicatesPerLayout's members due to the way static_vector's resize is implemented, which assumes that all std::array's elements are in an uninitialized state by default.

Changing static_vector to use assignment in resize instead of placement new/delete fixes the issue without adding a bunch of extra plumbing or running the destructor over each element in static_vector's constructor.
Copy link

github-actions bot commented Jun 19, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@loopunit
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@apanteleev apanteleev merged commit 6945d2a into NVIDIAGameWorks:main Jul 3, 2024
5 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 2024
@loopunit loopunit deleted the Fixing-static_vector branch July 4, 2024 06:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants