Skip to content

Commit

Permalink
Rollup merge of rust-lang#80488 - CAD97:drop-weak-without-reference, …
Browse files Browse the repository at this point in the history
…r=m-ou-se

Do not create dangling &T in Weak<T>::drop

Since at this point all strong pointers have been dropped, the wrapped `T` has also been dropped. As such, creating a `&T` to the dropped place is negligent at best (language UB at worst). Since we have `Layout::for_value_raw` now, use that instead of `Layout::for_value` to avoid creating the `&T`.

This does have implications for custom (potentially thin) DSTs, though much less severe than those discussed in rust-lang#80407. Specifically, one of two things has to be true:

- It has to be possible to use a `*const T` to a dropped (potentially custom, potentially thin) unsized tailed object to determine the layout (size/align) of the object. This is what is currently implemented (though with `&T` instead of `&T`). The validity of reading some location after it has been dropped is an open question IIUC (rust-lang/unsafe-code-guidelines#188) (except when the whole type is `Copy`, per `drop_in_place`'s docs).
  In this design, custom DSTs would get a `*mut T` and use that to return layout, and must be able to do so while in the "zombie" (post-drop, pre-free) state.
- `RcBox`/`ArcInner` compute and store layout eagerly, so that they don't have to ask the type for its layout after dropping it.

Importantly, this is already true today, as you can construct `Rc<DST>`, create a `Weak<DST>`, and drop the `Rc` before the `Weak`. This PR is a strict improvement over the status quo, and the above question about potentially thin DSTs will need to be resolved by any custom DST proposal.
  • Loading branch information
m-ou-se committed Dec 30, 2020
2 parents 46111c1 + 81685e9 commit 3d93dfd
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2042,7 +2042,7 @@ impl<T: ?Sized> Drop for Weak<T> {
// the strong pointers have disappeared.
if inner.weak() == 0 {
unsafe {
Global.deallocate(self.ptr.cast(), Layout::for_value(self.ptr.as_ref()));
Global.deallocate(self.ptr.cast(), Layout::for_value_raw(self.ptr.as_ptr()));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1927,7 +1927,7 @@ impl<T: ?Sized> Drop for Weak<T> {

if inner.weak.fetch_sub(1, Release) == 1 {
acquire!(inner.weak);
unsafe { Global.deallocate(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())) }
unsafe { Global.deallocate(self.ptr.cast(), Layout::for_value_raw(self.ptr.as_ptr())) }
}
}
}
Expand Down

0 comments on commit 3d93dfd

Please sign in to comment.