5 ms·
In Rust, this destructor is perfectly fine: impl Drop for Foo { fn drop(..) { self.cleanup(); self.free_memory(); }
by fluffything 7y ago
In Rust, this destructor is perfectly fine:
impl Drop for Foo {
fn drop(..) {
self.cleanup();
self.free_memory();
}
}
even if `self.cleanup()` panics. When that happens, `self.free_memory` will never be called, and memory will be leaked if `panic=unwind`, which is the default behavior. If there is a `catch_unwind` somewhere, these leaks will grow over time.
The same code in C++:
~foo() {
this->cleanup();
this->free_memory();
}
does not have this issue, because if `this->cleanup()` unwinds, that unwind makes `~foo` abort, because `~foo` is `noexcept(true)` by default.
In Rust, one cannot make `Drop::drop` be "nounwind", so to emulate C++'s behavior, one needs to write:
impl Drop for Foo {
fn drop(...) {
if let Err(_) = cath_unwind(|| {
self.cleanup(); self.free_memory();
}) {
abort();
}
}
}
or use a `DropGuard` to make sure that `self.free_memory()` is called if `self.cleanup` throws, etc.
Destructors being fallible are very weird things. In C++, destructors are infallible (at least by default). Rust supports fallible destructors, with all its cost (every time an object is dropped, doing this can fail, adding another "return" point from your function), yet minimal or zero value.
At least, I couldn't find any examples in the Rust book / reference / rust-by-example that show in which situations unwinding from a destructor is a good thing to do.
- status_quo69 7y agoUnless I'm misunderstanding your comment, you can make panics abort rather than unwind. https://doc.rust-lang.org/edition-guide/rust-2018/error-handling-and-panics/aborting-on-panic.html https://doc.rust-lang.org/edition-guide/rust-2018/error-hand...
- imron 7y agoYou are misunderstanding the comment, because it's specifically talking about the case when panic is set to unwind and there is catch_unwind handler.
- jstrong 7y agoif `self.cleanup()` panics, the program crashes, though? How is a memory leak even a relevant concept in that case?
- andrewflnr 7y agoOnly if someone specifically catches the panic, averting the crash. That's what the catch_unwind stuff was about.
- steveklabnik 7y ago> In Rust, this destructor is perfectly fine: Sort of; it's very rare. You don't really write free_memory, unless you're doing some very specific unsafe things. > When that happens, `self.free_memory` will never be called, and memory will be leaked if `panic=unwind`, which is the default behavior. It will be leaked if panic=unwind and if you use catch_panic. catch_panic is not used very much. Partially because panic=abort is also a valid semantic, and partially because if this Drop impl is called when panic happens, regardless of panic settings, you'll get an abort if self.cleanup() panics. > Rust supports fallible destructors, This isn't really true. Fallability in Rust is spelled "Result<T, E>", and drop does not return one. You're talking about a non-recoverable error, which, it is true, Rust does not give you tools to prevent, really. That's because it's very, very rarely used, because the whole intention is to end the current thread of execution. > At least, I couldn't find any examples in the Rust book / reference / rust-by-example that show in which situations unwinding from a destructor is a good thing to do. There isn't, which is why it isn't done, which is why your point confused me :)
- jynelson 7y agoWell, rarely done isn't the same as never done. docs.rs has exactly the tradeoffs the parent comment is talking about: we have a long-running daemon thread that uses `catch_unwind` and builder threads that occasionally panic. We've had [issues with memory leaks](https://github.com/rust-lang/docs.rs/issues/656 https://github.com/rust-lang/docs.rs/issues/656) in the past - they weren't related to unwinding that I know of, but it's still possible that they were. However I'm not in favor of the proposed solution - if the docs.rs server aborted every time a thread panicked we would have a lot of outages!
- steveklabnik 7y ago> Well, rarely done isn't the same as never done. Absolutely. But I do think the difference between "idiomatic" and "rarely done" is valuable, here. And as you said yourself, it's not clear that these leaks are caused by this kind of thing. If someone saw Rust code leaking memory all the time due to catching panics, I'd want to know about it, because it's very contradictory to my own experience, and I think that's interesting.
- inamberclad 7y agoHuh, I didn't know about catch_unwind. Very cool