Repository: rust-lang-deprecated/failure (language: rust)

Issue:
Memory leak in `Error::downcast` in failure 0.1.2
The following code quickly leaks memory:
```rust
#[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`](https://docs.rs/crate/failure/0.1.2/source/src/error/error_impl.rs) 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.


Discussion / hints:
I can't think of a good way to fix this without making the `Inner` contain the `Fail` as `Option` :(
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.

Fix patch:
diff --git a/Cargo.toml b/Cargo.toml
index d8c505a..029280e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -7,6 +7,7 @@ license = "MIT OR Apache-2.0"
 name = "failure"
 repository = "https://github.com/rust-lang-nursery/failure"
 version = "0.1.2"
+build = "build.rs"
 
 [dependencies.failure_derive]
 optional = true
diff --git a/build.rs b/build.rs
new file mode 100644
index 0000000..06de694
--- /dev/null
+++ b/build.rs
@@ -0,0 +1,39 @@
+use std::env;
+use std::process::Command;
+use std::str;
+use std::str::FromStr;
+
+fn main() {
+    if rustc_has_global_alloc() {
+        println!("cargo:rustc-cfg=has_global_alloc");
+    }
+}
+
+fn rustc_has_global_alloc() -> bool {
+    let rustc = match env::var_os("RUSTC") {
+        Some(rustc) => rustc,
+        None => return false,
+    };
+
+    let output = match Command::new(rustc).arg("--version").output() {
+        Ok(output) => output,
+        Err(_) => return false,
+    };
+
+    let version = match str::from_utf8(&output.stdout) {
+        Ok(version) => version,
+        Err(_) => return false,
+    };
+
+    let mut pieces = version.split('.');
+    if pieces.next() != Some("rustc 1") {
+        return true;
+    }
+
+    let next = match pieces.next() {
+        Some(next) => next,
+        None => return false,
+    };
+
+    u32::from_str(next).unwrap_or(0) >= 28
+}
diff --git a/src/error/error_impl.rs b/src/error/error_impl.rs
index f033c56..57eb896 100644
--- a/src/error/error_impl.rs
+++ b/src/error/error_impl.rs
@@ -1,4 +1,3 @@
-use core::mem;
 use core::ptr;
 
 use Fail;
@@ -49,8 +48,23 @@ impl ErrorImpl {
         });
         match ret {
             Some(ret) => {
-                // forget self (backtrace is dropped, failure is moved
-                mem::forget(self);
+                // deallocate the box without dropping the inner parts
+                #[cfg(has_global_alloc)] {
+                    use std::alloc::{dealloc, Layout};
+                    unsafe {
+                        let layout = Layout::for_value(&*self.inner);
+                        let ptr = Box::into_raw(self.inner);
+                        dealloc(ptr as *mut u8, layout);
+                    }
+                }
+
+                // slightly leaky versions of the above thing which makes the box
+                // 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);
+                }
+
                 Ok(ret)
             }
             _ => Err(self)
diff --git a/src/error/mod.rs b/src/error/mod.rs
index 04c0303..23df427 100644
--- a/src/error/mod.rs
+++ b/src/error/mod.rs
@@ -122,6 +122,9 @@ impl Error {
     /// failure is of the type `T`. For this reason it returns a `Result` - in
     /// the case that the underlying error is of a different type, the
     /// original `Error` is returned.
+    ///
+    /// Note that this method leaks on Rust versions < 1.28.0.
+    #[cfg_attr(not(has_global_alloc), deprecated(note = "this method leaks on Rust versions < 1.28"))]
     pub fn downcast<T: Fail>(self) -> Result<T, Error> {
         self.imp.downcast().map_err(|imp| Error { imp })
     }
@@ -225,4 +228,11 @@ mod test {
         drop(error);
         assert!(true);
     }
+
+    #[test]
+    fn test_downcast() {
+        let error: Error = io::Error::new(io::ErrorKind::NotFound, "test").into();
+        let real_io_error = error.downcast_ref::<io::Error>().unwrap();
+        assert_eq!(real_io_error.to_string(), "test");
+    }
 }


Test patch: