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

Memory leak in `Error::downcast` in failure 0.1.2 #261

Closed
jonas-schievink opened this Issue Sep 27, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@jonas-schievink
Copy link

jonas-schievink commented Sep 27, 2018

The following code quickly leaks memory:

#[macro_use]
extern crate failure;

use failure::Error;

#[derive(Debug, Fail)]
#[fail(display = "my custom failure type")]
struct MyFail;

fn main() {
    loop {
        Error::from(MyFail).downcast::<MyFail>().unwrap();
    }
}

This happens because Error::downcast does not drop the Box<Inner<Fail>> contained within the Error, it only drops the backtrace contained in that box and moves out the error type itself.

(Thanks to @rustonaut for finding the root cause!)

Note that I've not tested this on master, but on 0.1.2, which is the most up-to-date version published on Crates.io.

@mitsuhiko

This comment has been minimized.

Copy link
Collaborator

mitsuhiko commented Sep 29, 2018

I can't think of a good way to fix this without making the Inner contain the Fail as Option :(

@mitsuhiko

This comment has been minimized.

Copy link
Collaborator

mitsuhiko commented Sep 29, 2018

Actually this cannot be fixed with making it an Option and accessing the deallocators is impossible on 1.18 which this is tested against. Taking suggestions on how to fix this.

@rustonaut

This comment has been minimized.

Copy link
Contributor

rustonaut commented Oct 1, 2018

It might be a bit late but I think I found a way to do it in 1.18 without any magic.

Basically you can deref "move" Boxed content (e.g.: let _: String = *Box::new("x".to_owned());).

So it should be probably possible to do something like this:

  1. check if the Error contains a T
  2. cast the box back the the original non-DST type used to create the DST Box
  3. use box deref to move the thing back on the stack
  4. use destruction to get the parts back out (we should be able to no longer
    need any ptr reads)
@mitsuhiko

This comment has been minimized.

Copy link
Collaborator

mitsuhiko commented Oct 1, 2018

@rustonaut if you want to give this a try I would love to pull a PR that implements this!

@rustonaut

This comment has been minimized.

Copy link
Contributor

rustonaut commented Oct 1, 2018

Sure, I will see when I can fit it in.
(Should be in the next one or two days).

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