Repository navigation
mem::forget is unsafe, but can be written in entirely safe code #24456
Description
Activity
Nominating for 1.0 because some men just want to watch the world burn.
This is one of the root causes of #24292
Hmm... I was trying to understand this example, but it doesn't compile:
test.rs:6:25: 6:34 error: mismatched types: expected `core::option::Option<_>`, found `alloc::rc::Rc<(T, core::cell::RefCell<core::option::Option<_>>)>` (expected enum `core::option::Option`, found struct `alloc::rc::Rc`) [E0308] test.rs:6 *x.1.borrow_mut() = x.clone(); ^~~~~~~~~My first guess was to wrap
x.clone()intoSome(x.clone())so that the types matches, but it doesn't compile either:test.rs:6:25: 6:40 error: mismatched types: expected `core::option::Option<_>`, found `core::option::Option<alloc::rc::Rc<(T, core::cell::RefCell<core::option::Option<_>>)>>` (cyclic type of infinite size) [E0308] test.rs:6 *x.1.borrow_mut() = Some(x.clone()); ^~~~~~~~~~~~~~~Here is a working example:
fn forget<T>(val: T) { use std::cell::RefCell; use std::rc::Rc; struct Foo<T>(T, RefCell<Option<Rc<Foo<T>>>>); let x = Rc::new(Foo(val, RefCell::new(None))); *x.1.borrow_mut() = Some(x.clone()); } struct DontDropMe; impl Drop for DontDropMe { fn drop(&mut self) { unreachable!() } } fn main() { forget(DontDropMe) }
Reacted by zohnannorMy current thinking:
- There were other ways to forget even before Rc, at least in some cases. For example, if
T:Sendholds, you could send the value to a thread that runs an infinite loop, or which is deadlocked on a port. - It is true that one cannot assume that a destructor will run, and hence that
forgetis not itself unsafe (rather, it is unsafe to write a dtor that must run). This is what happened with std::thread::JoinGuard (and scoped) are unsound because of reference cycles #24292. You can sidestep this by writing destructors that must run but never giving up ownership of the value with the destructor. This often involves some closures. - It'd be nice if we could use RAII as we initially wanted to. So I suspect we may in the future work on a solution for identifying types whose destructors must run and preventing them from being placed into
Rc. (Some kind of trait, most likely.) - If we did step 2, then we would probably want
forgetto continue to "forget" the values in question -- which would mean thatforgetwould become unsafe again. So that argues in favor of leaving it asunsafefor now -- we can always make it safe later if we so choose.
Reacted by Nayuki and João Marcos- There were other ways to forget even before Rc, at least in some cases. For example, if
@nikomatsakis Did you mean "4. If we did step 3, [...]" rather than "4. If we did step 2, [...]"?
@nikomatsakis My concern is that (3) couldn't be done backwards-compatibly, at least not without making it unsafe to create a new type that leaks like
Rcdoes. Doing it now would subsume all four points (except point [1], but as I mentioned earlier Rust is a long way off from providing guarantees of totality or anything crazy like that, so it will be sufficient in practice).If you have
T: 'static, then you can move it into a thread-localVec<Box<SomeTrait>>(with animpl<T:?Sized> SomeTrait for T {}) or something and lose it, soforgetis only "interesting" on lifetime-bounded traits, where it allows the lifetime to end without the destructor running.Reacted by StudentA safe
mem::forgethas the advantage that it makes it easier to write the counterexample provingthread::JoinGuardunsafe. Safemem::forgetmakes it more likely that people will know that destructors are not realiable, so they can avoid repeating this mistake.we can always make it safe later if we so choose.
Making an unsafe function safe is a (minor) breaking change (
fn(T)is not a subtype ofunsafe fn(T)).We might want two functions:
fn forget<T>(val: T) { ... } unsafe fn forget_unsafe<T: ?Leak>(val: T) { ... }I commented on #24292, but the gist is that there are multiple ways to leak memory today (e.g. #14875 and #16135), so a targeted solution at
Rcmay not cover all use cases. Although as I mention in #24292 these other bugs can also be considered separate bugs on their own which need to be fixed regardless (but sometimes is quite difficult to do so).I have created an RFC for marking
forgetas a safe function.By the way, is it okay to leak references through
Rc-powered forget?fn forget<T>(val: T) { use std::cell::RefCell; use std::rc::Rc; struct Foo<T>(T, RefCell<Option<Rc<Foo<T>>>>); let x = Rc::new(Foo(val, RefCell::new(None))); *x.1.borrow_mut() = Some(x.clone()); } fn main() { { let mut a = 10; forget(&mut a); // Leak the reference to `a` let b = &mut a; // Now we have two mut references to `a`, one here and one somewhere } // The dangling reference to `a` still lives somewhere. Can it be made accessible without `unsafe`? }closing this issue to encourage all further discussion of this matter to happen on the RFC thread, see rust-lang/rfcs#1066
@reem the idea was to funnel conversation over to the RFC. A single comment in the middle of a thread won't do that.
(To be clear: this was was the outcome of the team triage, since the issue had been nominated; it was was a group decision. My initial instinct had been to let it sit for a week with the nomination, but was persuaded by arguments to close in favor of the RFC.)
For instance:
This is problematic, and an indicator that something is amiss in our rules.
cc @aturon @nikomatsakis @bstrie from IRC