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 downcast #262

Merged
merged 1 commit into from Oct 1, 2018

Conversation

Projects
None yet
2 participants
@mitsuhiko
Copy link
Collaborator

mitsuhiko commented Sep 29, 2018

This fixes #261 on newer Rust versions. I don't know of a way to fix it for olders.

The deprecation note for older Rusts is also the only way I know in which a user could be informed about this issue.

// itself leak. There is no good way around this as far as I know.
#[cfg(not(has_global_alloc))] {
use core::mem;
mem::forget(self);

This comment has been minimized.

@little-dude

little-dude Oct 1, 2018

Contributor

This is pure curiosity, but do you mind explaining why this leaks memory?

If I understand correctly, the ptr::read(fail as *const T) does a memcopy T and moves T out of its box without deallocating the box. But when we to mem::forget(self) why isn't the box's memory not deallocated?

This comment has been minimized.

@mitsuhiko

mitsuhiko Oct 1, 2018

Collaborator

mem::forget tells it not to run a destructor. By moving out the fields all inner fields were destroyed but the heap allocation for the containing struct in the box is forgotten and leaks.

@mitsuhiko mitsuhiko merged commit 345cb5a into master Oct 1, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@mitsuhiko mitsuhiko deleted the bugfix/fix-leak branch Oct 1, 2018

rustonaut added a commit to 1aim/failure that referenced this pull request Oct 2, 2018

Fix memory leak in downcast for older rust versions (rust-lang-nurser…
…y#262).

The previous fix didn't work for all supported
rust versions as it was relying on the alloc fn's.

rustonaut added a commit to 1aim/failure that referenced this pull request Oct 2, 2018

Fix memory leak in downcast for older rust versions (rust-lang-nurser…
…y#262).

The previous fix didn't work for all supported
rust versions as it was relying on the alloc fn's.

mitsuhiko added a commit that referenced this pull request Oct 20, 2018

Fix memory leak in downcast for older rust versions (#262). (#265)
The previous fix didn't work for all supported
rust versions as it was relying on the alloc fn's.

sticnarf added a commit to sticnarf/donkey-fs that referenced this pull request Oct 30, 2018

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