diff --git a/src/libcore/mem/mod.rs b/src/libcore/mem/mod.rs index 1cf2b40e93068..dac9ee6a5d91e 100644 --- a/src/libcore/mem/mod.rs +++ b/src/libcore/mem/mod.rs @@ -58,7 +58,9 @@ pub use crate::intrinsics::transmute; /// /// # Examples /// -/// Leak an I/O object, never closing the file: +/// The canonical safe use of `mem::forget` is to circumvent a value's destructor +/// implemented by the `Drop` trait. For example, this will leak a `File`, i.e. reclaim +/// the space taken by the variable but never close the underlying system resource: /// /// ```no_run /// use std::mem; @@ -68,9 +70,40 @@ pub use crate::intrinsics::transmute; /// mem::forget(file); /// ``` /// -/// The practical use cases for `forget` are rather specialized and mainly come -/// up in unsafe or FFI code. However, [`ManuallyDrop`] is usually preferred -/// for such cases, e.g.: +/// This is useful when the ownership of the underlying resource was previously +/// transferred to code outside of Rust, for example by transmitting the raw +/// file descriptor to C code. +/// +/// # Relationship with `ManuallyDrop` +/// +/// While `mem::forget` can also be used to transfer *memory* ownership, doing so is error-prone. +/// [`ManuallyDrop`] should be used instead. Consider, for example, this code: +/// +/// ``` +/// use std::mem; +/// +/// let mut v = vec![65, 122]; +/// // Build a `String` using the contents of `v` +/// let s = unsafe { String::from_raw_parts(v.as_mut_ptr(), v.len(), v.capacity()) }; +/// // leak `v` because its memory is now managed by `s` +/// mem::forget(v); // ERROR - v is invalid and must not be passed to a function +/// assert_eq!(s, "Az"); +/// // `s` is implicitly dropped and its memory deallocated. +/// ``` +/// +/// There are two issues with the above example: +/// +/// * If more code were added between the construction of `String` and the invocation of +/// `mem::forget()`, a panic within it would cause a double free because the same memory +/// is handled by both `v` and `s`. +/// * After calling `v.as_mut_ptr()` and transmitting the ownership of the data to `s`, +/// the `v` value is invalid. Even when a value is just moved to `mem::forget` (which won't +/// inspect it), some types have strict requirements on their values that +/// make them invalid when dangling or no longer owned. Using invalid values in any +/// way, including passing them to or returning them from functions, constitutes +/// undefined behavior and may break the assumptions made by the compiler. +/// +/// Switching to `ManuallyDrop` avoids both issues: /// /// ``` /// use std::mem::ManuallyDrop; @@ -80,24 +113,24 @@ pub use crate::intrinsics::transmute; /// // does not get dropped! /// let mut v = ManuallyDrop::new(v); /// // Now disassemble `v`. These operations cannot panic, so there cannot be a leak. -/// let ptr = v.as_mut_ptr(); -/// let cap = v.capacity(); +/// let (ptr, len, cap) = (v.as_mut_ptr(), v.len(), v.capacity()); /// // Finally, build a `String`. -/// let s = unsafe { String::from_raw_parts(ptr, 2, cap) }; +/// let s = unsafe { String::from_raw_parts(ptr, len, cap) }; /// assert_eq!(s, "Az"); /// // `s` is implicitly dropped and its memory deallocated. /// ``` /// -/// Using `ManuallyDrop` here has two advantages: +/// `ManuallyDrop` robustly prevents double-free because we disable `v`'s destructor +/// before doing anything else. `mem::forget()` doesn't allow this because it consumes its +/// argument, forcing us to call it only after extracting anything we need from `v`. Even +/// if a panic were introduced between construction of `ManuallyDrop` and building the +/// string (which cannot happen in the code as shown), it would result in a leak and not a +/// double free. In other words, `ManuallyDrop` errs on the side of leaking instead of +/// erring on the side of (double-)dropping. /// -/// * We do not "touch" `v` after disassembling it. For some types, operations -/// such as passing ownership (to a function like `mem::forget`) requires them to actually -/// be fully owned right now; that is a promise we do not want to make here as we are -/// in the process of transferring ownership to the new `String` we are building. -/// * In case of an unexpected panic, `ManuallyDrop` is not dropped, but if the panic -/// occurs before `mem::forget` was called we might end up dropping invalid data, -/// or double-dropping. In other words, `ManuallyDrop` errs on the side of leaking -/// instead of erring on the side of dropping. +/// Also, `ManuallyDrop` prevents us from having to "touch" `v` after transferring the +/// ownership to `s` - the final step of interacting with `v` to dispoe of it without +/// running its destructor is entirely avoided. /// /// [drop]: fn.drop.html /// [uninit]: fn.uninitialized.html