From b4edc3ca61c1f1a98052ec8ffc41d1b533b08fd7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 13 Dec 2019 19:05:09 +0100 Subject: [PATCH] Don't leak exceptions --- nix-rust/src/error.rs | 20 ++++++++++++++++++-- src/libutil/rust-ffi.cc | 6 +++++- src/libutil/rust-ffi.hh | 31 +++++++++++++++++++++++++------ 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/nix-rust/src/error.rs b/nix-rust/src/error.rs index 586a8f53d..85ac926e9 100644 --- a/nix-rust/src/error.rs +++ b/nix-rust/src/error.rs @@ -76,7 +76,7 @@ impl From for CppException { fn from(err: Error) -> Self { match err { Error::Foreign(ex) => ex, - _ => unsafe { make_error(&err.to_string()) }, + _ => CppException::new(&err.to_string()), } } } @@ -85,7 +85,23 @@ impl From for CppException { #[derive(Debug)] pub struct CppException(*const libc::c_void); // == std::exception_ptr* +impl CppException { + fn new(s: &str) -> Self { + Self(unsafe { make_error(s) }) + } +} + +impl Drop for CppException { + fn drop(&mut self) { + unsafe { + destroy_error(self.0); + } + } +} + extern "C" { #[allow(improper_ctypes)] // YOLO - fn make_error(s: &str) -> CppException; + fn make_error(s: &str) -> *const libc::c_void; + + fn destroy_error(exc: *const libc::c_void); } diff --git a/src/libutil/rust-ffi.cc b/src/libutil/rust-ffi.cc index accc5e22b..6f36b3192 100644 --- a/src/libutil/rust-ffi.cc +++ b/src/libutil/rust-ffi.cc @@ -3,10 +3,14 @@ extern "C" std::exception_ptr * make_error(rust::StringSlice s) { - // FIXME: leak return new std::exception_ptr(std::make_exception_ptr(nix::Error(std::string(s.ptr, s.size)))); } +extern "C" void destroy_error(std::exception_ptr * ex) +{ + free(ex); +} + namespace rust { std::ostream & operator << (std::ostream & str, const String & s) diff --git a/src/libutil/rust-ffi.hh b/src/libutil/rust-ffi.hh index 1466eabec..4fecce606 100644 --- a/src/libutil/rust-ffi.hh +++ b/src/libutil/rust-ffi.hh @@ -152,28 +152,47 @@ struct Source template struct Result { - unsigned int tag; + enum { Ok = 0, Err = 1, Uninit = 2 } tag; union { T data; std::exception_ptr * exc; }; + Result() : tag(Uninit) { }; // FIXME: remove + + Result(const Result &) = delete; + + Result(Result && other) + : tag(other.tag) + { + other.tag = Uninit; + if (tag == Ok) + data = std::move(other.data); + else if (tag == Err) + exc = other.exc; + } + ~Result() { - if (tag == 0) + if (tag == Ok) data.~T(); - else if (tag == 1) - // FIXME: don't leak exc + else if (tag == Err) + free(exc); + else if (tag == Uninit) ; + else + abort(); } /* Rethrow the wrapped exception or return the wrapped value. */ T unwrap() { - if (tag == 0) + if (tag == Ok) { + tag = Uninit; return std::move(data); - else if (tag == 1) + } + else if (tag == Err) std::rethrow_exception(*exc); else abort();