lang (unsafe | typesystem | machine)
Alter the signature of the std::mem::forget function to remove unsafe.
Explicitly state that it is not considered unsafe behavior to not run
destructors.
It was recently discovered by @arielb1 that the thread::scoped
API was unsound. To recap, this API previously allowed spawning a child thread
sharing the parent's stack, returning an RAII guard which join'd the child
thread when it fell out of scope. The join-on-drop behavior here is critical to
the safety of the API to ensure that the parent does not pop the stack frames
the child is referencing. Put another way, the safety of thread::scoped relied
on the fact that the Drop implementation for JoinGuard was always run.
The underlying issue for this safety hole was that it is possible
to write a version of mem::forget without using unsafe code (which drops a
value without running its destructor). This is done by creating a cycle of Rc
pointers, leaking the actual contents. It has been pointed out
that Rc is not the only vector of leaking contents today as there are
known bugs where panic! may fail to run
destructors. Furthermore, it has also been pointed out that not
running destructors can affect the safety of APIs like Vec::drain_range in
addition to thread::scoped.
It has never been a guarantee of Rust that destructors for a type will run, and
this aspect was overlooked with the thread::scoped API which requires that its
destructor be run! Reconciling these two desires has lead to a good deal of
discussion of possible mitigation strategies for various aspects of this
problem. This strategy proposed in this RFC aims to fit uninvasively into the
standard library to avoid large overhauls or destabilizations of APIs.
Primarily, the unsafe annotation on the mem::forget function will be
removed, allowing it to be called from safe Rust. This transition will be made
possible by stating that destructors may not run in all circumstances (from
both the language and library level). The standard library and the primitives it
provides will always attempt to run destructors, but will not provide a
guarantee that destructors will be run.
It is still likely to be a footgun to call mem::forget as memory leaks are
almost always undesirable, but the purpose of the unsafe keyword in Rust is to
indicate memory unsafety instead of being a general deterrent for "should be
avoided" APIs. Given the premise that types must be written assuming that their
destructor may not run, it is the fault of the type in question if mem::forget
would trigger memory unsafety, hence allowing mem::forget to be a safe
function.
Note that this modification to mem::forget is a breaking change due to the
signature of the function being altered, but it is expected that most code will
not break in practice and this would be an acceptable change to cherry-pick into
the 1.0 release.
It is clearly a very nice feature of Rust to be able to rely on the fact that a
destructor for a type is always run (e.g. the thread::scoped API). Admitting
that destructors may not be run can lead to difficult API decisions later on and
even accidental unsafety. This route, however, is the least invasive for the
standard library and does not require radically changing types like Rc or
fast-tracking bug fixes to panicking destructors.
The main alternative this proposal is to provide the guarantee that a destructor for a type is always run and that it is memory unsafe to not do so. This would require a number of pieces to work together:
Rc and Arc types would need be reevaluated somehow. One option would
be to statically prevent cycles, and another option would be to disallow types
that are unsafe to leak from being placed in Rc and Arc (more details
below).There has been quite a bit of discussion specifically on the topic of Rc and
Arc as they may be tricky cases to fix. Specifically, the compiler could
perform some form of analysis could to forbid all cycles or just those that
would cause memory unsafety. Unfortunately, forbidding all cycles is likely to
be too limiting for Rc to be useful. Forbidding only "bad" cycles, however, is
a more plausible option.
Another alternative, as proposed by @arielb1, would be a Leak marker
trait to indicate that a type is "safe to leak". Types like Rc would
require that their contents are Leak, and the JoinGuard type would opt-out
of it. This marker trait could work similarly to Send where all types are
considered leakable by default, but types could opt-out of Leak. This
approach, however, requires Rc and Arc to have a Leak bound on their type
parameter which can often leak unfortunately into many generic contexts (e.g.
trait objects). Another option would be to treat Leak more similarly to
Sized where all type parameters have a Leak bound by default. This change
may also cause confusion, however, by being unnecessarily restrictive (e.g. all
collections may want to take T: ?Leak).
Overall the changes necessary for this strategy are more invasive than admitting destructors may not run, so this alternative is not proposed in this RFC.
Are there remaining APIs in the standard library which rely on destructors being run for memory safety?